Skip to content

Conversation

cheenamalhotra
Copy link
Member

Ports #3620 to release/6.1

…3620)

* Fix SetProvider to return immediately if user-defined provider found

* Include test

* Fix tests

* Remove unwanted changes

* Update config file name

* Rename file back to app.config

* Copy always

* Disable tests for now.

* Fix framework inclusion

* Fix test failures

* Touch ups

* Fix test (continued)
@cheenamalhotra cheenamalhotra requested a review from a team as a code owner October 3, 2025 16:48
@Copilot Copilot AI review requested due to automatic review settings October 3, 2025 16:48
@cheenamalhotra cheenamalhotra changed the title Fix SetProvider to return immediately if user-defined provider found [6.1] Fix SetProvider to return immediately if user-defined provider found Oct 3, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR ports a fix to the SetProvider method in the SqlAuthenticationProviderManager to return immediately when a user-defined provider is found, preventing it from being replaced. The fix ensures that custom authentication providers configured via app.config files are preserved and not overridden by default providers.

Key changes:

  • Modified SetProvider method to return false immediately when a user-defined provider exists
  • Added comprehensive test coverage for app.config provider registration scenarios
  • Enhanced documentation with comments about configuration file support limitations

Reviewed Changes

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

Show a summary per file
File Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlAuthenticationProviderManager.cs Core fix: changed break to return false to prevent overriding user-defined providers
src/Microsoft.Data.SqlClient/tests/FunctionalTests/app.config Added test configuration to register a dummy authentication provider
src/Microsoft.Data.SqlClient/tests/FunctionalTests/DataCommon/DummySqlAuthenticationProvider.cs Created dummy provider for testing app.config registration
src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlAuthenticationProviderTest.cs Added test to verify user-defined provider is used instead of default
src/Microsoft.Data.SqlClient/tests/FunctionalTests/AADAuthenticationTests.cs Added integration test to validate dummy provider functionality
src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.FunctionalTests.csproj Updated project to include new files and copy app.config for .NET Framework builds
Comments suppressed due to low confidence (1)

src/Microsoft.Data.SqlClient/tests/FunctionalTests/DataCommon/DummySqlAuthenticationProvider.cs:1

  • The assertion parameters are in the wrong order. In xUnit, the expected value should come first: Assert.Equal(DummySqlAuthenticationProvider.DUMMY_TOKEN_STR, token.AccessToken).
// Licensed to the .NET Foundation under one or more agreements.

@cheenamalhotra cheenamalhotra added this to the 6.1.2 milestone Oct 3, 2025
@cheenamalhotra cheenamalhotra merged commit 2a7cfd2 into release/6.1 Oct 3, 2025
247 of 251 checks passed
@cheenamalhotra cheenamalhotra deleted the dev/cheena/6.1-fix-provider-registration branch October 3, 2025 19:22
Copy link

codecov bot commented Oct 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.09%. Comparing base (7a7b54e) to head (547dfcb).
⚠️ Report is 13 commits behind head on release/6.1.

Additional details and impacted files
@@               Coverage Diff               @@
##           release/6.1    #3651      +/-   ##
===============================================
+ Coverage        69.69%   70.09%   +0.39%     
===============================================
  Files              281      279       -2     
  Lines            62413    61748     -665     
===============================================
- Hits             43500    43283     -217     
+ Misses           18913    18465     -448     
Flag Coverage Δ
addons 90.82% <ø> (ø)
netcore 73.11% <0.00%> (+0.35%) ⬆️
netfx 69.79% <100.00%> (+0.55%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants