Adding token refresh hooks for VS Code account-based Entra MFA auth#21986
Open
Benjin wants to merge 22 commits into
Open
Adding token refresh hooks for VS Code account-based Entra MFA auth#21986Benjin wants to merge 22 commits into
Benjin wants to merge 22 commits into
Conversation
… dev/benjin/tokenRefreshHooks
This reverts commit 368d6ea.
PR Changes
|
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (41.66%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #21986 +/- ##
==========================================
- Coverage 74.83% 74.67% -0.16%
==========================================
Files 394 394
Lines 120403 120786 +383
Branches 7197 7207 +10
==========================================
+ Hits 90102 90196 +94
- Misses 30301 30590 +289
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <copilot@github.com>
Benjin
requested review from
aasimkhan30,
allancascante,
caohai,
kburtram,
laurenastrid1,
lewis-sanchez and
ssreerama
as code owners
Benjin
commented
May 6, 2026
Benjin
commented
May 6, 2026
Benjin
commented
May 6, 2026
Benjin
commented
May 7, 2026
Benjin
commented
May 7, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Adds extension-side support for SQLToolsService (STS) to request/refresh Entra MFA access tokens when using VS Code account-based auth, aligning with the paired STS PR.
Changes:
- Introduces new STS notifications for token refresh requests/responses and wires a handler in
ConnectionManager. - Updates STS launch arguments and security token request handling to support client-driven token acquisition (VS Code accounts path) plus new telemetry action.
- Bumps bundled STS version and changes the preview setting default for VS Code accounts Entra MFA.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| extensions/mssql/test/unit/connectionManager.test.ts | Skips an existing VS Code accounts token test (no replacement added). |
| extensions/mssql/src/sharedInterfaces/telemetry.ts | Adds a new telemetry action for VS Code account token acquisition failures. |
| extensions/mssql/src/models/contracts/connection.ts | Adds new account/refreshToken and account/tokenRefreshed notification contracts. |
| extensions/mssql/src/models/contracts/azure.ts | Extends security token request/response contracts with optional account/tenant and expiry. |
| extensions/mssql/src/languageservice/serviceclient.ts | Switches STS launch flags based on the VS Code accounts preview feature. |
| extensions/mssql/src/controllers/connectionManager.ts | Registers/implements refresh-token notification handling and adds VS Code accounts branch for security token requests. |
| extensions/mssql/src/configurations/config.ts | Updates the SQLToolsService version to a newer build. |
| extensions/mssql/package.json | Changes mssql.preview.useVscodeAccountsForEntraMFA default from null to true. |
Comment on lines
+661
to
+669
| const resource = | ||
| params.resource ?? | ||
| getCloudProviderSettings(account.key.providerId).settings.sqlResource!; | ||
|
|
||
| const refreshedToken = await self.azureController.refreshAccessToken( | ||
| account, | ||
| self.accountStore, | ||
| params.tenantId, | ||
| resource as any, // TODO: fix type mismatch |
Comment on lines
+2245
to
+2272
| if (params.accountId) { | ||
| this._logger.verbose("VS Code accounts token request received"); | ||
| try { | ||
| const tokenInfo = await acquireSqlAccessTokenFromVscodeAccount( | ||
| params.accountId, | ||
| params.tenantId, | ||
| ); | ||
| this._logger.verbose("VS Code accounts token acquired successfully"); | ||
| return { | ||
| accountKey: params.accountId, | ||
| token: tokenInfo.token.token, | ||
| expiresOn: tokenInfo.token.expiresOn, | ||
| }; | ||
| } catch (error) { | ||
| this._logger.error( | ||
| `VS Code accounts token acquisition failed: ${getErrorMessage(error)}`, | ||
| ); | ||
| sendErrorEvent( | ||
| TelemetryViews.ConnectionManager, | ||
| TelemetryActions.AcquireVsCodeAccountToken, | ||
| error instanceof Error ? error : new Error(getErrorMessage(error)), | ||
| /* includeErrorMessage */ false, | ||
| /* errorCode */ undefined, | ||
| /* errorType */ undefined, | ||
| {}, | ||
| ); | ||
| return { accountKey: "", token: "", expiresOn: 0 }; | ||
| } |
aasimkhan30
previously approved these changes
May 8, 2026
Comment on lines
+664
to
+672
| const resource = | ||
| params.resource ?? | ||
| getCloudProviderSettings(account.key.providerId).settings.sqlResource!; | ||
|
|
||
| const refreshedToken = await self.azureController.refreshAccessToken( | ||
| account, | ||
| self.accountStore, | ||
| params.tenantId, | ||
| resource as any, // TODO: fix type mismatch |
Comment on lines
+638
to
+646
| if ( | ||
| previewService.isFeatureEnabled(PreviewFeature.UseVscodeAccountsForEntraMFA) | ||
| ) { | ||
| const tokenInfo = await acquireTokenFromVscodeAccountForResource( | ||
| getCloudResourceEndpoint("sqlResource"), | ||
| params.accountId, | ||
| params.tenantId, | ||
| ); | ||
| token = tokenInfo.token.token; |
| // Return null to signal "sign in" was selected (account is undefined on that item) | ||
| resolve(account ?? null); // eslint-disable-line no-restricted-syntax | ||
| // eslint-disable-next-line no-restricted-syntax | ||
| resolve(selectedItem.value ?? null); |
Comment on lines
+641
to
+642
| const tokenInfo = await acquireTokenFromVscodeAccountForResource( | ||
| getCloudResourceEndpoint("sqlResource"), |
Comment on lines
+664
to
+672
| const resource = | ||
| params.resource ?? | ||
| getCloudProviderSettings(account.key.providerId).settings.sqlResource!; | ||
|
|
||
| const refreshedToken = await self.azureController.refreshAccessToken( | ||
| account, | ||
| self.accountStore, | ||
| params.tenantId, | ||
| resource as any, // TODO: fix type mismatch |
Comment on lines
+159
to
+169
| export function getCloudResourceEndpoint(endpoint: keyof IProviderResources): string { | ||
| const cloudSettings = getCloudProviderSettings(); | ||
| const resource = cloudSettings.settings[endpoint]; | ||
|
|
||
| if (!resource) { | ||
| throw new Error( | ||
| locConstants.Azure.noResourceConfiguredForCurrentCloud( | ||
| endpoint, | ||
| cloudSettings.displayName, | ||
| ), | ||
| ); |
Comment on lines
+698
to
+706
| const resource = | ||
| params.resource ?? | ||
| getCloudProviderSettings(account.key.providerId).settings.sqlResource!; | ||
|
|
||
| const refreshedToken = await self.azureController.refreshAccessToken( | ||
| account, | ||
| self.accountStore, | ||
| params.tenantId, | ||
| resource as any, // TODO: fix type mismatch |
Comment on lines
+651
to
+672
| if (!params.accountId) { | ||
| self._logger?.verbose( | ||
| `Cannot refresh token: no accountId provided in refresh request for URI ${params.uri}`, | ||
| ); | ||
| sendErrorEvent( | ||
| TelemetryViews.ConnectionManager, | ||
| TelemetryActions.RefreshTokenNotification, | ||
| new Error("Missing accountId in refresh token notification"), | ||
| true, // includeErrorMessage | ||
| "missingAccountId", | ||
| undefined, | ||
| { | ||
| useVscodeAccountsForEntraMFA: String( | ||
| useVscodeAccountsForEntraMFA, | ||
| ), | ||
| }, | ||
| { | ||
| currentTimestamp: Math.floor(Date.now() / 1000), | ||
| }, | ||
| ); | ||
| return; | ||
| } |
Comment on lines
+768
to
+781
| currentTimestamp: Math.floor(Date.now() / 1000), | ||
| refreshedTokenExpirationTimestamp: | ||
| expiresOn !== undefined ? expiresOn : -1, | ||
| }, | ||
| ); | ||
|
|
||
| self.client.sendNotification( | ||
| ConnectionContracts.TokenRefreshedNotification.type, | ||
| { | ||
| token: token, | ||
| expiresOn: expiresOn ?? -1, | ||
| uri: params.uri, | ||
| }, | ||
| ); |
Comment on lines
+2513
to
+2515
| throw new Error( | ||
| `Failed to acquire token for account ${accountId} and tenant ${tenantId} (undefined result)`, | ||
| ); |
Comment on lines
+625
to
+650
| /** | ||
| * Handles the account/refreshToken notification from the service. | ||
| * Acquires a fresh token using VS Code accounts or MSAL, then sends | ||
| * account/tokenRefreshed back to the service. | ||
| */ | ||
| public handleRefreshTokenNotification(): NotificationHandler<ConnectionContracts.RefreshTokenParams> { | ||
| const self = this; | ||
| return (params: ConnectionContracts.RefreshTokenParams): void => { | ||
| void (async () => { | ||
| const useVscodeAccountsForEntraMFA = previewService.isFeatureEnabled( | ||
| PreviewFeature.UseVscodeAccountsForEntraMFA, | ||
| ); | ||
|
|
||
| try { | ||
| let token: string | undefined; | ||
| let expiresOn: number | undefined; | ||
|
|
||
| if (useVscodeAccountsForEntraMFA) { | ||
| const tokenInfo = await acquireTokenFromVscodeAccountForResource( | ||
| getCloudResourceEndpoint("sqlResource"), | ||
| params.accountId, | ||
| params.tenantId, | ||
| ); | ||
| token = tokenInfo.token.token; | ||
| expiresOn = tokenInfo.token.expiresOn; | ||
| } else { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adding the MSSQL-side changes for token refresh requests when VS Code accounts are being used for Entra MFA auth
Paired with microsoft/sqltoolsservice#2665
Code Changes Checklist
npm run test)Reviewers: Please read our reviewer guidelines