Remove sql auth provider flag#21523
Merged
aasimkhan30 merged 3 commits intomainfrom Mar 13, 2026
Merged
Conversation
aasimkhan30
requested review from
Benjin,
allancascante,
caohai,
kburtram,
laurenastrid1,
lewis-sanchez,
manujoseph85 and
ssreerama
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the user-facing setting/feature-flag for the SQL Authentication Provider and makes the extension always launch SQL Tools Service with the provider enabled, aligning the extension’s behavior with the now-required auth mode described in the PR.
Changes:
- Removed the
mssql.enableSqlAuthenticationProvidersetting from package contributions, docs, constants, and localization sources. - Always passes
--enable-sql-authentication-providerwhen launching SQL Tools Service. - Simplified MSAL controller logic by removing the conditional “provider enabled” branching and related unit test coverage.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| localization/xliff/vscode-mssql.xlf | Removes the trans-unit for the deleted setting description. |
| extensions/mssql/package.json | Removes the mssql.enableSqlAuthenticationProvider configuration contribution. |
| extensions/mssql/package.nls.json | Removes the localized description string for the deleted setting. |
| extensions/mssql/README.md | Removes the setting from documentation examples. |
| extensions/mssql/src/languageservice/serviceclient.ts | Always enables SQL Auth Provider via STS command-line argument. |
| extensions/mssql/src/constants/constants.ts | Removes the constant for the deleted setting key. |
| extensions/mssql/src/controllers/mainController.ts | Stops treating the deleted setting as requiring a reload prompt. |
| extensions/mssql/src/azure/utils.ts | Removes the config reader for the deleted setting. |
| extensions/mssql/src/azure/constants.ts | Removes constants tied to the deleted setting. |
| extensions/mssql/src/azure/azureController.ts | Removes provider-enabled state gating and related token-refresh branching. |
| extensions/mssql/src/azure/msal/msalAzureController.ts | Removes provider-enabled branching; init becomes a no-op; changes token acquisition behavior in populateAccountProperties. |
| extensions/mssql/src/azure/fileEncryptionHelper.ts | Always notifies STS of encryption key changes (no longer conditional on the removed setting). |
| extensions/mssql/test/unit/msalAzureController.test.ts | Removes unit test tied to the deleted setting/config. |
| extensions/mssql/test/unit/objectExplorerService.test.ts | Updates stubbing to match removed controller API/behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
324
to
+328
| let account = await this.addAccount(accountStore); | ||
| profile.user = account!.displayInfo.displayName; | ||
| profile.email = account!.displayInfo.email; | ||
| profile.accountId = account!.key.id; | ||
|
|
||
| // Skip fetching access token for profile if Sql Authentication Provider is enabled. | ||
| if (!this.isSqlAuthProviderEnabled()) { | ||
| if (!profile.tenantId) { | ||
| await this.promptForTenantChoice(account!, profile); | ||
| } | ||
|
|
||
| const token = await this.getAccountSecurityToken(account!, profile.tenantId, settings); | ||
|
|
||
| if (!token) { | ||
| let errorMessage = LocalizedConstants.msgGetTokenFail; | ||
| this.logger.error(errorMessage); | ||
| this._vscodeWrapper.showErrorMessage(errorMessage); | ||
| } else { | ||
| profile.azureAccountToken = token.token; | ||
| profile.expiresOn = token.expiresOn; | ||
| } | ||
| } else { | ||
| this.logger.verbose( | ||
| "SQL Authentication Provider is enabled, access token will not be acquired by extension.", | ||
| ); | ||
| } | ||
| this.logger.verbose( |
There was a problem hiding this comment.
populateAccountProperties no longer populates profile.azureAccountToken/profile.expiresOn (or tenantId) and just logs/returns. Callers such as ConnectionManager.confirmEntraTokenValidity use this method as the fallback path after a refresh failure and then copy profile.azureAccountToken/expiresOn back onto the connection; with this change that fallback will always leave the connection without a token and can cause repeated auth failures. Either keep this method acquiring and setting the token fields (and tenant choice) or update the fallback logic to not depend on populateAccountProperties for token acquisition in SQL Auth Provider mode.
PR Changes
|
aasimkhan30
changed the title
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (7.14%) 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 #21523 +/- ##
==========================================
+ Coverage 72.69% 72.72% +0.03%
==========================================
Files 330 330
Lines 98395 98298 -97
Branches 5455 5453 -2
==========================================
- Hits 71529 71491 -38
+ Misses 26866 26807 -59
🚀 New features to boost your workflow:
|
Benjin
reviewed
Mar 13, 2026
Benjin
approved these changes
Mar 13, 2026
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
The provider has been enabled by default for a long time, and the extension now effectively depends on that mode. In particular, when the SQL Authentication Provider is disabled, the extension no longer implements the token refresh contract expected by STS (language service), which can lead to broken or inconsistent authentication behavior.
Code Changes Checklist
npm run test)Reviewers: Please read our reviewer guidelines