Add AOT-safe authentication provider registration#4195
Add AOT-safe authentication provider registration#4195chweidling wants to merge 6 commits intodotnet:mainfrom
Conversation
SqlAuthenticationProviderManager discovers the Azure extension at startup via Assembly.Load + Activator.CreateInstance. Under NativeAOT these reflection APIs are trimmed away, silently leaving all Active Directory auth methods without a provider and causing 'Cannot find an authentication provider' errors at runtime. This change adds an AOT-safe bridge between the Abstractions and core assemblies: - SqlAuthenticationProvider.RegisterProviderManager(): called by the core SqlAuthenticationProviderManager during its static initialization to wire up direct (non-reflection) GetProvider/SetProvider callbacks. Providers that were registered before the manager initialized are buffered and replayed. - ActiveDirectoryAuthenticationProvider.RegisterAsDefault(): AOT-safe entry point that explicitly registers the MSAL-based provider for all 9 AD auth methods. This replaces the reflection-based discovery under NativeAOT. The existing non-AOT code path is unchanged: LoadAzureExtensionProvider() still runs after RegisterProviderManager() and uses reflection as before. Fixes dotnet#4193
|
Could a maintainer please add the Public API 🆕 label? This PR introduces two new public methods. |
There was a problem hiding this comment.
Pull request overview
This PR adds an AOT-safe mechanism for registering and discovering SqlAuthenticationProvider implementations by wiring direct GetProvider/SetProvider callbacks from SqlAuthenticationProviderManager into the Extensions.Abstractions layer, and introduces an explicit ActiveDirectoryAuthenticationProvider.RegisterAsDefault() entry point for NativeAOT apps to register the Azure provider without reflection.
Changes:
- Register AOT-safe provider manager callbacks during
SqlAuthenticationProviderManagerstatic initialization and isolate reflection-based Azure-provider discovery intoLoadAzureExtensionProvider()with trimming/AOT annotations. - Add
SqlAuthenticationProvider.RegisterProviderManager(...)plus buffering/replay of providers registered before the manager initializes. - Add
ActiveDirectoryAuthenticationProvider.RegisterAsDefault(...)and update/add tests to validate the new wiring and buffering behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlAuthenticationProviderManagerTests.cs | Adds a unit test to confirm the provider manager registers callback delegates into SqlAuthenticationProvider. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlAuthenticationProviderManager.cs | Registers callback bridge and extracts/annotates the reflection-based Azure provider discovery method. |
| src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs | Adds RegisterAsDefault() to explicitly register the Azure AD provider without reflection (AOT-safe). |
| src/Microsoft.Data.SqlClient.Extensions/Abstractions/test/SqlAuthenticationProviderTest.cs | Updates a test expectation to reflect new buffering behavior when core SqlClient assembly isn’t present. |
| src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationProvider.cs | Adds callback registration + pending-provider buffering/replay; changes GetProvider/SetProvider behavior to prefer callbacks. |
| /// <summary> | ||
| /// Providers registered via <see cref="SetProvider"/> before the core | ||
| /// SqlClient assembly has registered its provider manager callbacks | ||
| /// via <see cref="RegisterProviderManager"/>. Replayed once the | ||
| /// manager registers. | ||
| /// </summary> | ||
| private static Dictionary<SqlAuthenticationMethod, | ||
| SqlAuthenticationProvider>? s_pendingProviders; | ||
|
|
||
| private static Func<SqlAuthenticationMethod, | ||
| SqlAuthenticationProvider?>? s_getProviderCallback; | ||
|
|
||
| private static Func<SqlAuthenticationMethod, | ||
| SqlAuthenticationProvider, bool>? s_setProviderCallback; |
There was a problem hiding this comment.
s_pendingProviders is a mutable Dictionary accessed from SetProvider and RegisterProviderManager without any synchronization. Since SqlAuthenticationProvider.SetProvider is a public API and can be called concurrently (including before the manager callback is registered), this can cause races and dictionary corruption. Consider protecting pending provider buffering/replay with a lock, or using a ConcurrentDictionary plus an atomic swap when replaying.
There was a problem hiding this comment.
Let's the maintainers check this.
| return s_setProviderCallback( | ||
| authenticationMethod, provider); |
There was a problem hiding this comment.
With the new direct callback path, SqlAuthenticationProvider.SetProvider can now throw exceptions from the core manager (e.g., when provider.IsSupported(authenticationMethod) is false) instead of returning false as before. The public docs for SetProvider describe failure via a bool return and don’t mention exceptions; previously, the reflection-based path swallowed TargetInvocationException and returned false. Consider wrapping the callback invocation in a try/catch and translating failures back to false to preserve the existing public API behavior.
| return s_setProviderCallback( | |
| authenticationMethod, provider); | |
| try | |
| { | |
| return s_setProviderCallback( | |
| authenticationMethod, provider); | |
| } | |
| catch | |
| { | |
| return false; | |
| } |
There was a problem hiding this comment.
Let the maintainers check this.
The existing Internal.SetProvider(reflection path) also throws SQL.UnsupportedAuthenticationByProvider when IsSupported returns false. The callback points to the same SqlAuthenticationProviderManager.SetProvider method. Behavior is identical. Swallowing exceptions with a bare catch { return false; } would silently hide real bugs.
| public static bool SetProvider( | ||
| SqlAuthenticationMethod authenticationMethod, | ||
| SqlAuthenticationProvider provider) | ||
| { | ||
| return Internal.SetProvider(authenticationMethod, provider); | ||
| // Prefer the direct callback registered by the core SqlClient | ||
| // assembly (AOT-safe). | ||
| if (s_setProviderCallback is not null) | ||
| { | ||
| return s_setProviderCallback( | ||
| authenticationMethod, provider); | ||
| } | ||
|
|
||
| // The core SqlClient assembly has not initialized its | ||
| // callbacks yet. Buffer the registration and replay it once | ||
| // RegisterProviderManager is called. | ||
| s_pendingProviders ??= new(); | ||
| s_pendingProviders[authenticationMethod] = provider; | ||
| return true; |
There was a problem hiding this comment.
SqlAuthenticationProvider.SetProvider no longer falls back to the existing reflection-based bridge (Internal.SetProvider) when the callback isn’t registered. This breaks compatibility with older Microsoft.Data.SqlClient versions that don’t call RegisterProviderManager: SetProvider will return true (buffered) but the provider will never be applied, leading to runtime "Cannot find an authentication provider" errors. Consider attempting Internal.SetProvider first (non-AOT path), and only buffering when reflection is unavailable (e.g., MDS assembly not found / trimmed).
There was a problem hiding this comment.
Is this really the case?
The Abstractions and Core packages are always versioned and shipped together. There's no scenario where a newer Abstractions runs with an older Core that doesn't call RegisterProviderManager. And if Internal.SetProvider (reflection) worked, we wouldn't need the patch at all — the whole point is that reflection is broken under AOT.
|
@dotnet-policy-service agree company="DocuWare GmbH" |
…uthenticationProviderManager.cs Let's not break the build. The important information is there anyway. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…uthenticationProviderManager.cs Fix the comment Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| // The core SqlClient assembly has not initialized its | ||
| // callbacks yet. Buffer the registration and replay it once | ||
| // RegisterProviderManager is called. | ||
| s_pendingProviders ??= new(); | ||
| s_pendingProviders[authenticationMethod] = provider; | ||
| return true; | ||
| } |
There was a problem hiding this comment.
When the callback isn’t registered yet, SetProvider now buffers the provider and returns true without attempting the existing reflection-based Internal.SetProvider path. This changes the documented meaning of the return value (it may never actually be registered) and can break scenarios where a newer Abstractions assembly is used with an older core Microsoft.Data.SqlClient that never calls RegisterProviderManager. Consider first using the reflection path when available (preserving the real success/failure result), and only buffering when the core assembly / reflected method truly isn’t available.
There was a problem hiding this comment.
Abstractions and Core are co-versioned inside the same Microsoft.Data.SqlClient NuGet package — there's no supported scenario where a newer Abstractions runs against an older Core that never calls RegisterProviderManager. The reflection path (Internal.SetProvider) wouldn't help here either: it internally does Assembly.Load + MethodInfo.Invoke, which either triggers the Core static ctor (making s_setProviderCallback non-null anyway) or fails under AOT. Buffering is the correct behavior for the only real window where the callback is null — between Abstractions init and Core init.
Adds SetProvider_BufferedAndReplayed test that verifies providers registered via SetProvider before RegisterProviderManager is called are buffered and correctly replayed once manager callbacks are wired.
…uthenticationProviderManager.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
SetProvider_BufferedAndReplayed now snapshots s_getProviderCallback, s_setProviderCallback, and s_pendingProviders before the test and restores them in a finally block, preventing static state from leaking into other tests.
paulmedynski
added
the
Hotfix Candidate 🚑
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
paulmedynski
added
the
Regression 💥
|
There was a related discussion on the original PR #3680 here: we were talking about moving GetProvider() and SetProvider() from SqlAuthenticationProvider to SqlAuthenticationProviderManager. This was dropped at the time because it would have been a wider breaking change, but resulted in #3728. I feel that part of the underlying problem is that we've split the responsibility for maintaining authentication provider registrations between In this scenario, the
This would have the side-effect of enabling subsequent calls to SqlAuthenticationProvider.SetProvider to override the providers added from .config files, unless we added an overload to let users designate specific providers as "system providers" which can't be replaced. If we went down that route, I think we'd avert the need for |
paulmedynski
removed
the
Hotfix Candidate 🚑
|
This should be reviewed and tested on priority for 7.1.0-preview2. Do not release in 7.1.0-preview1 as it will also require Azure and Abstraction package releases - which will conflict with release MDS 7.0.x versions - as they can easily consume newer versions of Azure and Abstractions packages. cc @paulmedynski @mdaigle @benrr101 It's definitely a Hotfix Candidate for 7.0. |
cheenamalhotra
added
Hotfix Candidate 🚑
Description
SqlAuthenticationProviderManagerdiscovers the Azure extension at startup viaAssembly.Load+Activator.CreateInstance. Under NativeAOT these reflection APIs are trimmed away, silently leaving all Active Directory auth methods without a provider and causingCannot find an authentication provider for 'ActiveDirectoryManagedIdentity'errors at runtime.This change adds an AOT-safe bridge between the Abstractions and core assemblies:
SqlAuthenticationProvider.RegisterProviderManager(getProvider, setProvider): Called bySqlAuthenticationProviderManagerduring its static initialization to wire up direct (non-reflection)GetProvider/SetProvidercallbacks. Providers that were registered viaSetProviderbefore the manager initialized are buffered in a pending dictionary and replayed onceRegisterProviderManageris called. Marked[EditorBrowsable(Never)]since this is infrastructure, not application API.ActiveDirectoryAuthenticationProvider.RegisterAsDefault(): AOT-safe entry point that explicitly registers the MSAL-based provider for all 9 AD auth methods. This replaces the reflection-based discovery under NativeAOT. Applications call this early inMain()before opening any SQL connection.LoadAzureExtensionProvider(): Extracted from the static constructor into its own method and annotated with[RequiresUnreferencedCode]and[RequiresDynamicCode]so the trimmer/AOT analyzer can warn appropriately.The existing non-AOT code path is unchanged:
LoadAzureExtensionProvider()still runs afterRegisterProviderManager()and uses reflection as before.Usage under NativeAOT
Issues
Fixes #4193
Testing
CallbackDelegates_RegisteredByManager— verifies thatSqlAuthenticationProviderManager's static constructor registers the_getProviderCallbackand_setProviderCallbackfields onSqlAuthenticationProvider.SetProvider_NoMdsAssemblyin the Abstractions test project — with the new buffering behavior,SetProviderreturnstrue(buffered for replay) instead offalsewhen the core assembly hasn't loaded yet.PublishAot=true,net10.0,linux-arm64), connecting to Azure SQL with Managed Identity authentication. The service has been running in production successfully.Design Notes & Open Questions
This implementation prioritizes minimal diff and backwards compatibility. A few points worth discussing:
Thread safety of buffering:
s_pendingProvidersis a plainDictionarywithout synchronization. This is safe in practice because bothRegisterAsDefault()andRegisterProviderManager()run on the startup path (single-threaded, before any connections are opened), but it is not formally thread-safe. If reviewers prefer, this could be changed toConcurrentDictionaryor protected with a lock.Public API surface:
RegisterProviderManager()is technically public (though hidden with[EditorBrowsable(Never)]). An alternative would be a[ModuleInitializer]in the core assembly, or an internal method with[InternalsVisibleTo], but both have their own drawbacks (module initializers have ordering constraints;InternalsVisibleTocreates a tight coupling that breaks under strong-name signing differences). Open to feedback on the preferred approach.No guard against multiple calls:
RegisterProviderManager()can be called more than once, silently replacing the callbacks. A guard could be added, but the only legitimate caller isSqlAuthenticationProviderManager's static constructor, which runs exactly once.