Adding centralized cache for Entra tokens by Benjin · Pull Request #21434 · microsoft/vscode-mssql

Benjin · 2026-03-04T00:15:54Z

Description

Further improvements upon #20352

Originally, each connection instance had its own copy of the token spawned/copied from the Object Explorer node, so when that token expired and was refreshed, that refreshed token would live only within its owner.

#20352 improved this by updating the OE token copy when an editor spawned from it needed to reconnect, ensuring that future new editors get the updated token.

This change takes that a step further by sharing a token cache across all connections with the following behavior:

Each time a new connection is opened, it checks if the token currently attached is still valid.
If it's not valid, it checks to see if there's a valid token in the cache.
If there's no valid token in the cache, a new token is obtained, cached centrally in the connection manager, and attached to the current profile.

This way, when the user has multiple tabs open for the same connection, updated tokens are picked up from the cache to minimize the number of times a user is prompted for re-authentication.

Code Changes Checklist

New or updated unit tests added
All existing tests pass (npm run test)
Code follows contributing guidelines
Telemetry/logging updated if relevant
No regressions or UX breakage

Reviewers: Please read our reviewer guidelines

Copilot

Pull request overview

Adds a centralized, shared Entra (Azure MFA) SQL access-token cache in ConnectionManager so multiple editors/tabs using the same Entra account+tenant can reuse refreshed tokens and reduce repeated re-auth prompts.

Changes:

Introduces a shared Entra SQL token cache plus an “in-flight refresh” map to dedupe concurrent refreshes.
Replaces confirmEntraTokenValidity with refreshEntraTokenIfNeeded and wires it into relevant connection flows.
Adds unit tests covering cache usage, refresh behavior, coalescing parallel refreshes, and cache clearing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
extensions/mssql/src/controllers/connectionManager.ts	Implements shared Entra SQL token caching and coalesced refresh logic; integrates cache clearing.
extensions/mssql/test/unit/connectionManager.test.ts	Adds unit tests validating cache reuse, refresh, parallel coalescing, and cache clearing behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

extensions/mssql/src/controllers/connectionManager.ts

github-actions · 2026-03-04T00:26:12Z

PR Changes

Category	Target Branch	PR Branch	Difference
vscode-mssql VSIX	6392 KB	6393 KB	⚪ 1 KB ( 0% )
sql-database-projects VSIX	6119 KB	6119 KB	⚪ 0 KB ( 0% )
data-workspace VSIX	535 KB	535 KB	⚪ 0 KB ( 0% )

codecov-commenter · 2026-03-04T00:26:36Z

Codecov Report

❌ Patch coverage is 79.50000% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.35%. Comparing base (31eab5e) to head (4a6eb48).

Files with missing lines	Patch %	Lines
...ensions/mssql/src/controllers/connectionManager.ts	76.66%	35 Missing ⚠️
extensions/mssql/src/sharedInterfaces/webview.ts	77.77%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #21434      +/-   ##
==========================================
+ Coverage   73.25%   73.35%   +0.09%     
==========================================
  Files         328      328              
  Lines       99451    99588     +137     
  Branches     5649     5683      +34     
==========================================
+ Hits        72851    73050     +199     
+ Misses      26600    26538      -62

Files with missing lines	Coverage Δ
.../src/connectionSharing/connectionSharingService.ts	`98.93% <100.00%> (+<0.01%)`	⬆️
extensions/mssql/src/constants/locConstants.ts	`76.06% <100.00%> (+0.12%)`	⬆️
extensions/mssql/src/sharedInterfaces/webview.ts	`98.50% <77.77%> (-1.50%)`	⬇️
...ensions/mssql/src/controllers/connectionManager.ts	`62.43% <76.66%> (+4.46%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

extensions/mssql/src/controllers/connectionManager.ts

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

extensions/mssql/test/unit/connectionManager.test.ts

extensions/mssql/src/constants/locConstants.ts

extensions/mssql/src/sharedInterfaces/webview.ts

Copilot · 2026-03-20T15:52:03Z

extensions/mssql/src/controllers/connectionManager.ts

+                        token.onCancellationRequested(() => {
+                            reject({
+                                status: ApiStatus.Cancelled,
+                                message: "Azure sign in cancelled by user.",
+                            } as Status);
+                        });
+                        try {
+                            const refreshedToken = await refreshTask();
+                            resolve(refreshedToken);
+                        } catch (error) {
+                            const refreshErrorStatus: Status = {
+                                status: ApiStatus.Error,
+                                message: getErrorMessage(error),
+                            };
+                            reject(refreshErrorStatus);
+                        }


After cancellation triggers reject(...), the code can still proceed to refreshTask() and later call resolve(...)/reject(...) again (since the cancellation handler doesn’t short-circuit the async flow). This can lead to unnecessary interactive auth continuing even after the user cancels. Consider tracking a cancelled flag (set in the cancellation handler) and returning early before starting refreshTask(), and/or checking token.isCancellationRequested before and after the refresh call.

@copilot could you fix this?

@copilot Can you fix this?

@copilot open a new pull request to apply changes based on this feedback

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

extensions/mssql/src/controllers/connectionManager.ts

extensions/mssql/test/unit/connectionManager.test.ts

…h logic (#21721) * Initial plan * Handle undefined return from refreshAccessToken and add unit test Co-authored-by: Benjin <1609827+Benjin@users.noreply.github.com> Agent-Logs-Url: https://github.com/microsoft/vscode-mssql/sessions/1f79676e-8ac6-4a8f-b67c-1decd029bfb3 * Remove withProgressStub check from undefined token test Co-authored-by: Benjin <1609827+Benjin@users.noreply.github.com> Agent-Logs-Url: https://github.com/microsoft/vscode-mssql/sessions/dc0023a0-9d00-490e-bbc8-489393c6427f --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Benjin <1609827+Benjin@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

extensions/mssql/src/controllers/connectionManager.ts

Adding centralized cache for Entra tokens

5137f74

Copilot AI review requested due to automatic review settings March 4, 2026 00:15

Benjin requested review from aasimkhan30, allancascante, caohai, kburtram, laurenastrid1, lewis-sanchez, manujoseph85 and ssreerama as code owners March 4, 2026 00:15

Copilot started reviewing on behalf of Benjin March 4, 2026 00:17 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

aasimkhan30 reviewed Mar 4, 2026

View reviewed changes

extensions/mssql/src/controllers/connectionManager.ts Show resolved Hide resolved

aasimkhan30 reviewed Mar 4, 2026

View reviewed changes

extensions/mssql/src/controllers/connectionManager.ts Outdated Show resolved Hide resolved

aasimkhan30 reviewed Mar 4, 2026

View reviewed changes

extensions/mssql/src/controllers/connectionManager.ts Outdated Show resolved Hide resolved

Benjin added 2 commits March 5, 2026 22:26

PR feedback

bdca23e

Loc

c0e8b2f

Benjin modified the milestones: March 2026 Release, April 2026 Release Mar 6, 2026

Merge branch 'main' into dev/benjin/centralizedCache

469cded

Copilot AI review requested due to automatic review settings March 20, 2026 15:45

Copilot AI reviewed Mar 20, 2026

View reviewed changes

Adding more logging

42967d4

Copilot started reviewing on behalf of Benjin March 20, 2026 16:44 View session

aasimkhan30 previously approved these changes Mar 20, 2026

View reviewed changes

Benjin added 2 commits March 23, 2026 16:06

Merge branch 'main' into dev/benjin/centralizedCache

7ff77a6

PR feedback

527833c

Loc

4a2a27a

Copilot AI review requested due to automatic review settings March 23, 2026 23:25

Benjin dismissed aasimkhan30’s stale review via 4a2a27a March 23, 2026 23:25

Copilot started reviewing on behalf of Benjin March 23, 2026 23:26 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

extensions/mssql/src/controllers/connectionManager.ts Show resolved Hide resolved

extensions/mssql/src/controllers/connectionManager.ts Show resolved Hide resolved

extensions/mssql/test/unit/connectionManager.test.ts Show resolved Hide resolved

aasimkhan30 previously approved these changes Mar 23, 2026

View reviewed changes

This was referenced Mar 23, 2026

[WIP] [WIP] Addressing feedback on centralized cache for Entra tokens #21719

Closed

Handle undefined return from refreshAccessToken in Entra token refresh #21720

Closed

Handle undefined return from refreshAccessToken in Entra token refresh logic #21721

Merged

Benjin dismissed aasimkhan30’s stale review via c83f2e9 March 24, 2026 00:18

microsoft deleted a comment from Copilot AI Mar 24, 2026

Fixing test

4d8987a

Copilot AI review requested due to automatic review settings March 24, 2026 00:57

Copilot started reviewing on behalf of Benjin March 24, 2026 00:58 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

extensions/mssql/src/controllers/connectionManager.ts Show resolved Hide resolved

Merge branch 'main' into dev/benjin/centralizedCache

4a6eb48

Conversation

Benjin commented Mar 4, 2026

Description

Code Changes Checklist

Reviewers: Please read our reviewer guidelines

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Changes

Uh oh!

codecov-commenter commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Benjin Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Benjin Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Benjin Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Mar 4, 2026 •

edited

Loading

codecov-commenter commented Mar 4, 2026 •

edited

Loading