Commit Graph

228 Commits

Author SHA1 Message Date
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
4c32889faf Show InputBox for unsupported clients (#239389)
* Show InputBox for unsupported clients

Fixes https://github.com/microsoft/vscode/issues/238147

* comment

* Add 127.0.0.1 for good measure
2025-02-01 05:26:18 +00:00
Tyler James Leonhardt
a9ce0b5556 Include an ability to change the default client id (#238736)
Include an ability to change the client id

So our migration is easy to test
2025-01-24 16:46:50 -08:00
Tyler James Leonhardt
60230a46df Fix the booleans on the MSAL flows (#238148)
* Loopback does _not_ work in REH or WebWorker
* UrlHandler _does_ work in REH
2025-01-17 19:46:29 +01:00
Tyler James Leonhardt
8eddff6613 Default to MSAL authentication 🚀 (#237920)
Here we go. Ref https://github.com/microsoft/vscode/issues/178740
2025-01-14 21:23:14 +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
0a66dc39ff Adopt concept of flows in Microsoft Auth (#237006)
And only use Loopback flow when not running in Remote Extension Host.
2024-12-27 11:31:53 -08:00
Tyler James Leonhardt
358e96ab1e Cancel if the user dismisses the modal (#236642)
Fixes https://github.com/microsoft/vscode/issues/235364
2024-12-19 21:40:19 +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
Logan Ramos
e689b912ba Update telemetry package (#236378) 2024-12-17 20:12:49 +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
68cd78865a Remove msal-node-extensions workaround (#234456)
We needed this workaround because MSAL was always trying to require a native module we never use.

I sent a PR to MSAL to rework their behavior and that has now been released and we pulled that in in https://github.com/microsoft/vscode/pull/234450

With the updated msal-node-extensions library, we no longer need to do this webpack logic.
2024-11-23 00:39:17 +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
5cb3edbfc4 Move env.nativeHandle to window.nativeHandle (#234395) 2024-11-22 06:50:12 +05:30
Tyler James Leonhardt
d7ab35a98e handle: string to nativeHandle: UInt8Array based on feedback (#234378)
Feedback in https://github.com/microsoft/vscode/issues/229431
2024-11-21 11:58:49 -07:00
Tyler James Leonhardt
d12854f6ba Default MSAL to true (#234290)
Using new `microsoft-authentication.implementation` setting
2024-11-20 13:15:09 -08: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
b278635434 predictable keytar resolution (#233948)
In a couple of builds [like this one](https://dev.azure.com/monacotools/Monaco/_build/results?buildId=305323&view=results) we have seen bad comparisons of `extension.js` in the Microsoft Auth extension:

> 2024-11-15T19:16:07.080Z electron-universal SHA for file Contents/Resources/app/extensions/microsoft-authentication/dist/extension.js does not match across builds a4db653e84d42a8cb4681a2274dffd34e0d7729cf14c0c4228b668778aa81c18!=6ff1bd8b8b51db2bff1d5f000625f0efe490a92eb282d0559aa904325d6cad68

Which is odd, considering there is no native dependencies used on macOS. The suspicion is that this is because of `keytar` which we have in the package.json using an odd `file:./path...`syntax to prevent it from installing normally since we don't use it.

The solution: additionally alias keytar in the webpack config so that the resolution is predictable.
2024-11-15 21:47:48 +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
Raymond Zhao
fcae80e2df fix: useMsal setting doesn't have tags (#232534) 2024-10-29 15:26:18 -07:00
Tyler James Leonhardt
4ae34118f2 Reinstate normal loopback flow for Gov clouds (#230541)
I finally managed to make changes to the Gov cloud configurations and we can now use the loopback flow for the Gov clouds.
2024-10-04 22:22:00 +02:00
Robo
9494a89080 chore: add npmrc to extensions folder (#230214) 2024-10-01 12:43:01 +02:00
Tyler James Leonhardt
35183efe58 Revert "Rename MSAL setting" (#230164)
Revert "Rename MSAL setting (#230142)"

This reverts commit 2a677c686b.
2024-09-30 14:38:38 -07:00
Tyler James Leonhardt
2a677c686b Rename MSAL setting (#230142) 2024-09-30 19:25:47 +02:00
Tyler James Leonhardt
eef6488c2c Use select_account prompt when no account is specified (#229778)
Fixes https://github.com/microsoft/vscode/issues/226892
2024-09-25 21:44:09 -07: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
f26394d477 Use Electron fetch to get Proxy settings properly (#229202)
* Attempt to use Electron fetch

* Remove Content-Length header because electron doesn't like it

"Apparently Chromium doesn’t want the caller to set content-length, but will set it itself."
2024-09-20 20:24:41 +02:00
Tyler James Leonhardt
6bd8e90fb7 Misc fixes for Sovereign Clouds (#228591)
* Misc fixes for Sovereign Clouds

* For now, use the URL handler since the main flow doesn't work right now because the localhost redirect url needs to be in those environments
* Includes the name of the cloud in the PCAs so that we have separation between the auth providers
* extra logging for the URL Handler

* fix tests
2024-09-13 21:57:12 +02:00
Tyler James Leonhardt
db2a1df708 Use an EventBufferer to ensure only one event across PCAs (#228400) 2024-09-13 01:24:41 +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
hj
88d7b97c93 chore: rm unreached ignore items when build extensions. (#227906)
chore: rm unreached ignore items when compiling extensions.
2024-09-09 10:32:51 +00: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
Tyler James Leonhardt
9d28a232cb Bump MSAL Node (#227815) 2024-09-06 18:39:14 +02:00
Robo
b5a6aa14a8 feat: switch to npm as default package manager (#226927)
* feat: move from yarn to npm

* chore: skip yarn.lock files

* fix: playwright download

* chore: fix compile and hygiene

* chore: bump vsce@2.17.0

Refs 8b49e9dfdf

* test: update results for bat and sh colorizer tests

* fix: add missing lock files for windows

* fix: switch to legacy-peer-deps

* chore: update markdown-it@14.1.0

Refs 737c95a129

esbuild step in extensions-ci-pr was previously using markdown-it
from root which had userland punycode and was able to compile successfully.

* ci: increase pr timeout for windows integration tests

* chore: fix product build

* build: ignore extension dev dependency for rcedit

* build: fix working directory inside container

* build: fix dependency generation

* npm: update dependencies

* ci: use global npmrc

* ci: update cache

* ci: setup global npmrc for private npm auth

* build: fix extension bundling

* chore: sync npm dependencies

* ci: debug env variables for container

* ci: fix win32 cli pipeline

* build: fix npmrc config usage for build/ and remote/ dirs

* fix: windows build

* fix: container builds

* fix: markdown-language-features tests and bundling

```
[03:58:22] Error: Command failed: /Users/demohan/.nvm/versions/node/v20.15.1/bin/node /Users/demohan/github/vscode/extensions/markdown-language-features/esbuild-notebook.js --outputRoot /Users/demohan/github/vscode/.build/extensions/markdown-language-features
✘ [ERROR] Could not resolve "punycode"

    extensions/markdown-language-features/node_modules/markdown-it/lib/index.js:14:27:
      14 │ var punycode     = require('punycode');
         ╵                            ~~~~~~~~~~

  The package "punycode" wasn't found on the file system but is built into node. Are you trying to bundle for node? You can use "platform: 'node'" to do that, which will remove this error.
```

Adds userland package based on beed9aee2c

* fix: container builds for distro

* chore: update yarn occurrences

* fixup! chore: bump vsce@2.17.0

Uses the closest version to `main` branch that does not
include d3cc84cdec
while still having the fix 8b49e9dfdf

* chore: sync npm dependencies

* chore: sync npm dependencies

* chore: sync npm dependencies

* chore: throw error when yarn is used for installation

* chore: add review feedback

* chore: switch exec => run where needed

* chore: npm sync dependencies

* fix: markdown-language-features bundling

```
✘ [ERROR] Could not resolve "punycode"

    extensions/markdown-language-features/node_modules/markdown-it/lib/index.js:14:27:
      14 │ var punycode     = require('punycode');
         ╵                            ~~~~~~~~~~

  The package "punycode" wasn't found on the file system but is built into node. Are you trying to bundle for node? You can use "platform: 'node'" to do that, which will remove this error.
```

Adds missing userland package based on markdown-it/markdown-it@beed9ae,
can be removed once we update markdown-it >= 14.1.0

* ci: rename no-yarn-lock-changes.yml

* chore: sync npm dependencies

* ci: restore no-yarn-lock-changes.yml

We can disable it in a separate PR to keep the required
checks happy and also need workflow edit perms.

* chore: sync npm dependencies

* ci: rebuild cache

* ci: fix no-package-lock-changes.yml

* chore: bump distro

* chore: rm yarn.lock files

* chore: rm yarn.lock files without dependencies

* chore: add vscode-selfhost-import-aid to postinstall dirs

* chore: bump distro
2024-09-06 22:18:02 +09:00
Tyler James Leonhardt
214bf83a48 3 Changes to MSAL auth (#226580)
* Remove access token refreshing logic. The new calling pattern for an extension is that they should just always call `getSession` before doing something with it. The session that returns will be valid because MSAL will refresh any access tokens that are close to expiry using the refresh tokens that it has
  * NOTE: access tokens expire after 1hr. Refresh tokens expire after like... many days.
* Have `createSession` fire an `onDidChangeSession` event so that the badge goes away
* Improved logging messages
2024-08-26 08:27:20 +02:00
Tyler James Leonhardt
6544b003dc Bump msal-node and fix a bad contrast ratio (#226210)
Bumps MSAL-node which contains [my fix](https://github.com/AzureAD/microsoft-authentication-library-for-js/pull/7247) that will actually show our error template.

Also fixes an A11y contrast issue with said error template.
2024-08-22 00:53:21 +02:00
Tyler James Leonhardt
e1db90f81d Add logging for deciding on MSAL (#226112)
so it's clear why
2024-08-20 18:28:02 -07:00
Tyler James Leonhardt
c125b90d41 Add experimentation to Microsoft auth (#226107)
* Add experimentation to Microsoft auth

So that we can gradually roll out MSAL support.

* correct order
2024-08-20 17:17:15 -07:00
Tyler James Leonhardt
b2d6860308 Improve refresh and scope handling (#225832)
* Moves the `setupRefresh` stuff into the CachedPublicClientApp simplifying things a bit
* Uses a ScopeData class to handle all scope operations fixing an issue where we were passing in the wrong array into the `acquireTokenInteractive`
2024-08-17 07:55:33 +02:00
Tyler James Leonhardt
9d49135f49 LoopbackClientAndOpener Tests (#225458)
(first tests in this extension!)
2024-08-12 18:42:44 -07:00
Tyler James Leonhardt
44464d7ccd Fix reload modal logic when turning on/off MSAL (#225446)
This wasn't firing before. This fixes that.
2024-08-13 00:24:18 +02:00
Tyler James Leonhardt
a8b2cef91c Adopt error template and some minor tweaks to error rendering (#225426)
NOTE: For this to get lit up, this PR needs to go in in MSAL-node: https://github.com/AzureAD/microsoft-authentication-library-for-js/pull/7247
2024-08-12 20:05:04 +02:00
Tyler James Leonhardt
70d27743ac Ability to use MSAL in the Desktop (#225272)
* Ability to use MSAL in the Desktop

* add comment about MSAL workaround
2024-08-09 12:18:34 -07:00
Tyler James Leonhardt
02b638ae27 Finalize getAccounts API (#224877)
Fixes https://github.com/microsoft/vscode/issues/152399
2024-08-05 11:59:33 -07:00
Tyler James Leonhardt
86495e947b Use global fetch and crypto (#221736)
Now that we're on Node 20, we can just use the global fetch and crypto which work the same in node and in the browser.
2024-07-15 10:58:31 -07:00