Tests | Cleanup AAD Password auth from tests + code cleanup by cheenamalhotra · Pull Request #4390 · dotnet/SqlClient

cheenamalhotra · 2026-06-23T01:52:54Z

Follow up changes to test suite after #4288 where AAD Password auth was disabled from pipelines. This PR removes the AAD Password Conn String parameter completely to make it easier for AAD tests to be managed.

What Changed

Pipeline / config

Replace AAD_PASSWORD_CONN_STR(_eastus) with AZURE_SQL_CONN_STRING_WestUS2 / AZURE_SQL_CONN_STRING_EastUS (bare conn string, no credentials).
Remove AADAuthorityURL config property, as its now unused.
Rename config property AADPasswordConnectionString → AzureSqlConnectionString across Config.cs, config.default.jsonc, and pipeline templates (update-config-file-step.yml, ci-run-tests-job.yml, dotnet-sqlclient-ci-core.yml, test-azure-package-ci-job.yml).
Move AADServicePrincipalId inside the IsFork == 'False' guard (matches existing policy for AADServicePrincipalSecret).

New shared test helpers (tests/Common/)

CommonUtils.cs: EnsureSeparator, GenerateRandomSecureString, GenerateRandomCharacters, GenerateObjectName.
StringExtensions.cs: fluent Add*AuthenticationToConnString extensions (Integrated, Interactive, DeviceCodeFlow, ServicePrincipal, WorkloadIdentity, MSI, Default) plus AddUserToConnString / AddPasswordToConnString. Tests now compose auth onto AzureSqlConnectionString dynamically — required for broker-aware test legs.

Test updates

ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs: removed all ActiveDirectoryPassword cases; rewrote MSI / Workload Identity / Device Code / Default / token-callback tests against the new helpers; applied target-typed new(), collection expressions, using SecureString, Assert.Equal(ConnectionState.Open, …).
DataTestUtility.cs, AADFedAuthTokenRefreshTest, ConnectionPoolTest, and Microsoft.Data.SqlClient.Extensions/Azure/test/AADConnectionTest.cs + Config.cs updated to match.
TESTGUIDE.md and .github/instructions/testing.instructions.md updated.

Breaking change (CI only)

Downstream pipelines / config.user.jsonc must rename AADPasswordConnectionString → AzureSqlConnectionString and supply a bare Azure SQL connection string (no Authentication=, User ID=, Password=). The variables AAD_PASSWORD_CONN_STR / _eastus are no longer consumed and can be retired.

Validation

dotnet build ManualTests / net9.0 — 0 warnings, 0 errors.
dotnet test UnitTests / net9.0 — 792 pass; 4 failures are pre-existing macOS-env issues (Windows-only SSPI + keychain-restricted AE fixtures), not regressions.
Full PR pipeline (validates renamed Azure-SQL secrets).
Tests updated.
No public API changes (test-only).

Future TODOs

Propogate changes to all release branches.
Remove AAD_PASSWORD_CONN_STR, AAD_PASSWORD_CONN_STR_eastus, AADAuthorityURL and Azure_Password variables from library.

Copilot

Pull request overview

Removes the Entra ID “password auth” connection-string configuration from the test suite (and CI templates) following the pipeline change in #4288, replacing it with a single “bare” AzureSqlConnectionString that tests extend at runtime to exercise specific authentication modes.

Changes:

Renamed AADPasswordConnectionString → AzureSqlConnectionString across test config (Config.cs/config.default.jsonc), docs, and Azure DevOps pipeline templates; updated pipeline variables accordingly.
Added shared test helpers under tests/Common/ (CommonUtils, StringExtensions) to compose auth/credential keywords onto a base connection string.
Updated ManualTests + Azure extensions tests to remove ActiveDirectoryPassword coverage and to use managed identity / token-based flows with the new helpers.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
TESTGUIDE.md	Updates documented test config property name/meaning for AAD/Azure SQL tests.
src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/Microsoft.Data.SqlClient.TestUtilities.csproj	References the new shared test helper project.
src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/config.default.jsonc	Renames config field and clarifies Azure SQL base conn string expectation (no creds).
src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/Config.cs	Renames the config field to `AzureSqlConnectionString`.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs	Removes AAD password tests and rewrites AAD scenarios to use helpers and AzureSqlConnectionString.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs	Updates pooled AAD/token tests to use AzureSqlConnectionString and async token retrieval.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/AADFedAuthTokenRefreshTest/AADFedAuthTokenRefreshTest.cs	Switches the fed-auth refresh scenario to managed identity-based auth/provider usage.
src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/ManagedIdentityProvider.cs	Renames the custom provider class to clarify it targets user-assigned MI.
src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs	Renames config fields, adds MI-based token retrieval APIs, and removes AAD password token generation.
src/Microsoft.Data.SqlClient/tests/Common/StringExtensions.cs	Adds fluent helpers for composing auth/credential keywords and for removing auth/credential keywords.
src/Microsoft.Data.SqlClient/tests/Common/CommonUtils.cs	Adds shared random/name helpers used by updated tests.
src/Microsoft.Data.SqlClient.Extensions/Azure/test/Config.cs	Renames the Azure test config field read from config.jsonc.
src/Microsoft.Data.SqlClient.Extensions/Azure/test/AADConnectionTest.cs	Updates Azure extension tests to build auth modes from the base Azure SQL conn string.
eng/pipelines/jobs/test-azure-package-ci-job.yml	Switches pipeline config-writing to `AzureSqlConnectionString` and updates secret scoping.
eng/pipelines/dotnet-sqlclient-ci-core.yml	Switches CI template config-writing to `AzureSqlConnectionString` (WestUS2/EastUS).
eng/pipelines/common/templates/steps/update-config-file-step.yml	Renames template parameter and output property to `AzureSqlConnectionString`.
eng/pipelines/common/templates/jobs/ci-run-tests-job.yml	Renames config pass-through property to `AzureSqlConnectionString`.
.github/instructions/testing.instructions.md	Updates the test-config docs to match the renamed Azure SQL config property.

Copilot

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.

Copilot

Pull request overview

Copilot reviewed 28 out of 29 changed files in this pull request and generated 5 comments.

Files not reviewed (1)

src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

Copilot

Pull request overview

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

Files not reviewed (1)

src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

Copilot

Pull request overview

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

Files not reviewed (1)

src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

Copilot

Pull request overview

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

Files not reviewed (1)

src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

Copilot

Pull request overview

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

Files not reviewed (1)

src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

… SQL The ConnectionPoolConnectionStringProvider classes now add Authentication=ActiveDirectoryDefault when running against Azure SQL, so pool tests authenticate via managed identity instead of failing with 'Login failed for user ''' after auth properties are stripped. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

Files not reviewed (1)

src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

Comments suppressed due to low confidence (1)

src/Microsoft.Data.SqlClient.Extensions/Azure/test/Config.cs:48

Config defines both TCPConnectionString (populated from config) and TcpConnectionString (left as string.Empty). HasTcpConnectionString() and several tests use TcpConnectionString, which means those tests will be skipped or build connection strings from an empty base. Make TcpConnectionString an alias of TCPConnectionString (or remove it and update all call sites) so the configured value is used consistently.

    internal static string TCPConnectionString { get; } = string.Empty;
    internal static string ServicePrincipalId { get; } = string.Empty;
    internal static string ServicePrincipalSecret { get; } = string.Empty;
    internal static string SystemAccessToken { get; } = string.Empty;
    internal static bool SystemAssignedManagedIdentitySupported { get; } = false;
    internal static string TcpConnectionString { get; } = string.Empty;

…oryDefault Convert ConnectionPoolTest and TransactionPoolTest to async, using DataTestUtility.GetAccessTokenAsync() to set connection.AccessToken before opening connections targeting Azure SQL. This ensures pool tests authenticate via managed identity without embedding auth in the connection string (which would conflict with tests that set AccessToken or AccessTokenCallback directly). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 29 out of 30 changed files in this pull request and generated 5 comments.

Files not reviewed (1)

src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

Copilot

Pull request overview

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

Files not reviewed (1)

src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

Comments suppressed due to low confidence (1)

src/Microsoft.Data.SqlClient.Extensions/Azure/test/Config.cs:47

Config currently defines both TCPConnectionString and TcpConnectionString, but only TCPConnectionString is populated in the static constructor. HasTcpConnectionString() and some tests use TcpConnectionString, so they will see an empty string and get skipped/misconfigured. Collapse to a single backing property (keep TcpConnectionString) and make TCPConnectionString an alias if you still want the uppercase name for callers.

    internal static string TCPConnectionString { get; } = string.Empty;
    internal static string ServicePrincipalId { get; } = string.Empty;
    internal static string ServicePrincipalSecret { get; } = string.Empty;
    internal static string SystemAccessToken { get; } = string.Empty;
    internal static bool SystemAssignedManagedIdentitySupported { get; } = false;

            SystemAssignedManagedIdentitySupported =
                GetBool(root, "SupportsSystemAssignedManagedIdentity");
-            TcpConnectionString = GetString(root, "TCPConnectionString");
+            TCPConnectionString = GetString(root, "TCPConnectionString");


Copilot

Pull request overview

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

Files not reviewed (1)

src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

+    internal static bool IsAzureSqlConnectionString() => !TCPConnectionString.IsEmpty() && IsAzureSqlServer();
    internal static bool HasServicePrincipal() => !ServicePrincipalId.IsEmpty() && !ServicePrincipalSecret.IsEmpty();
    internal static bool HasSystemAccessToken() => !SystemAccessToken.IsEmpty();
    internal static bool HasTcpConnectionString() => !TcpConnectionString.IsEmpty();


Tests | Cleanup AAD Password auth from tests + code cleanup

bcbac46

Copilot AI review requested due to automatic review settings June 23, 2026 01:52

github-project-automation Bot added this to SqlClient Board Jun 23, 2026

github-project-automation Bot moved this to To triage in SqlClient Board Jun 23, 2026

cheenamalhotra added the Area\Tests Issues that are targeted to tests or test projects label Jun 23, 2026

Copilot started reviewing on behalf of cheenamalhotra June 23, 2026 01:53 View session

cheenamalhotra added this to the 7.1.0-preview2 milestone Jun 23, 2026

Copilot AI reviewed Jun 23, 2026

View reviewed changes

cheenamalhotra added 2 commits June 22, 2026 19:15

Fix build

62e88a8

Update references

18f879c

Copilot AI review requested due to automatic review settings June 23, 2026 06:56

Copilot started reviewing on behalf of cheenamalhotra June 23, 2026 06:56 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

cheenamalhotra added 2 commits June 23, 2026 00:07

More updates

8153957

Fix compiler errors for net462

29d4617

Copilot AI review requested due to automatic review settings June 23, 2026 08:11

Copilot started reviewing on behalf of cheenamalhotra June 23, 2026 08:11 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

cheenamalhotra added 2 commits June 23, 2026 14:50

Fix error

60262cd

Fix failing test

7119636

Copilot AI review requested due to automatic review settings June 24, 2026 02:42

Copilot started reviewing on behalf of cheenamalhotra June 24, 2026 02:42 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/tests/Common/CommonUtils.cs Outdated

Comment thread src/Microsoft.Data.SqlClient/tests/Common/StringExtensions.cs Outdated

Comment thread src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs

cheenamalhotra added 2 commits June 24, 2026 11:31

Fix tests

4185ce8

Fix TestCustomProviderAuthentication

f8f1c89

Copilot AI review requested due to automatic review settings June 24, 2026 20:22

Copilot started reviewing on behalf of cheenamalhotra June 24, 2026 20:22 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

Address comments

4b40fcc

cheenamalhotra marked this pull request as ready for review June 25, 2026 00:50

Copilot AI review requested due to automatic review settings June 26, 2026 21:02

Copilot started reviewing on behalf of cheenamalhotra June 26, 2026 21:02 View session

Copilot AI reviewed Jun 26, 2026

View reviewed changes

cheenamalhotra added 2 commits June 26, 2026 14:53

Update Azure config, cleanup

a535d7b

CI changes

f8e3829

Copilot AI review requested due to automatic review settings June 26, 2026 22:07

Copilot started reviewing on behalf of cheenamalhotra June 26, 2026 22:08 View session

Copilot AI reviewed Jun 26, 2026

View reviewed changes

paulmedynski modified the milestones: 7.1.0-preview2, 7.1.0-preview3 Jun 30, 2026

Copilot AI review requested due to automatic review settings July 10, 2026 06:09

Copilot started reviewing on behalf of cheenamalhotra July 10, 2026 06:09 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs

Comment thread eng/pipelines/common/templates/jobs/ci-run-tests-job.yml Outdated

Copilot AI review requested due to automatic review settings July 10, 2026 06:24

Copilot started reviewing on behalf of cheenamalhotra July 10, 2026 06:24 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Minor changes

843e033

Copilot AI review requested due to automatic review settings July 10, 2026 06:44

Copilot started reviewing on behalf of cheenamalhotra July 10, 2026 06:44 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient.Extensions/Azure/test/Config.cs

SystemAssignedManagedIdentitySupported =

GetBool(root, "SupportsSystemAssignedManagedIdentity");

TcpConnectionString = GetString(root, "TCPConnectionString");

TCPConnectionString = GetString(root, "TCPConnectionString");

Touch-ups

857b56e

Copilot AI review requested due to automatic review settings July 10, 2026 06:51

Copilot started reviewing on behalf of cheenamalhotra July 10, 2026 06:52 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

cheenamalhotra requested review from benrr101 and mdaigle July 10, 2026 06:57

cheenamalhotra moved this from Waiting for customer to In review in SqlClient Board Jul 10, 2026

cheenamalhotra marked this pull request as draft July 10, 2026 17:12

Uh oh!

Conversation

cheenamalhotra commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Changed

Breaking change (CI only)

Validation

Future TODOs

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

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!

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!

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 left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

cheenamalhotra commented Jun 23, 2026 •

edited

Loading