-
Notifications
You must be signed in to change notification settings - Fork 330
Enable WAM Broker support for Entra ID Auth modes #4288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
46 commits
Select commit
Hold shift + click to select a range
3db6009
Enable WAM Broker support for Entra ID Auth modes
cheenamalhotra 7a0a460
Add test connection project for validating WAM broker behavior on UI …
cheenamalhotra d23e4a6
Merge branch 'main' into dev/cheena/entra-wam-broker
cheenamalhotra bc5178a
Update redirectURI + minor changes
cheenamalhotra 240e5ce
Merge branch 'dev/cheena/entra-wam-broker' of https://github.com/dotn…
cheenamalhotra bca2a62
Comments/cleanup
cheenamalhotra 58083b6
Update config file
cheenamalhotra fd54502
Update Redirect URI for Unix
cheenamalhotra b657f32
Remove duplication
cheenamalhotra 01529eb
Fix unix build
cheenamalhotra 152b44e
Apply suggestions from code review
cheenamalhotra fff68be
Apply suggestions from code review
cheenamalhotra 17564ca
Apply suggestions from code review
cheenamalhotra 0d105e0
Update comment
cheenamalhotra 97fce03
useWamBroker option and tests
cheenamalhotra de30171
Merge branch 'main' into dev/cheena/entra-wam-broker
cheenamalhotra 84949bb
Update api and docs
cheenamalhotra 82a2d4e
handle .net standard
cheenamalhotra f14c637
Remove specs
cheenamalhotra 81312dc
Add collection, address docs
cheenamalhotra 5683852
fix: allow AzureSqlConnector fallback build on non-Windows
Copilot 3f91b03
Address feedback
cheenamalhotra d23b760
Address comments/more changes
cheenamalhotra 449d289
Verify Sibling Assembly
cheenamalhotra 90760e3
Deprecation note for future
cheenamalhotra 69b3787
Support clearing user token cache to enable retesting token acquisiti…
cheenamalhotra b44ece5
More improvements, clear token cache properly + fix device code flow …
cheenamalhotra b82106d
Remove temp file
cheenamalhotra 3064e77
Apply suggestions from code review
cheenamalhotra d436a77
Remove unwanted new ctors, fix errors
cheenamalhotra 6ef039e
Merge branch 'dev/cheena/entra-wam-broker' of https://github.com/dotn…
cheenamalhotra 8c89141
Address PR review: fix </param>, expand WamBroker tests, README note
cheenamalhotra ff4f373
More improvements + tests
cheenamalhotra da1929d
Last Updates
cheenamalhotra 681b64f
Update MSAL as well
cheenamalhotra e628e2b
Potential fix for pull request finding
cheenamalhotra f8a7c96
Merge branch 'main' into dev/cheena/entra-wam-broker
cheenamalhotra 7eac499
Removed unwanted changes
cheenamalhotra 744af8d
Missing doc
cheenamalhotra c003f4a
Final changes
cheenamalhotra 8a72de1
More updates, rename options type
cheenamalhotra ed46a31
Update provider instance initialization flow, other minor changes
cheenamalhotra b802a2e
Disable broker for AAD Password auth (not supported by MSAL unless us…
cheenamalhotra a63ac86
Revert "Disable broker for AAD Password auth (not supported by MSAL u…
cheenamalhotra 5fdca68
Remove AAD Password auth tests
cheenamalhotra 9e703ed
Fix wording
cheenamalhotra File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| # Feature Specification: WAM Broker Support for Entra ID Authentication | ||
|
|
||
| **Feature Branch**: `dev/automation/wam-broker-support` | ||
| **Created**: 2026-05-20 | ||
| **Status**: Draft | ||
| **References**: | ||
|
|
||
| - PR [#2884](https://github.com/dotnet/SqlClient/pull/2884) (original POC, closed) | ||
| - PR [#3874](https://github.com/dotnet/SqlClient/pull/3874) (updated POC, closed) | ||
| - ICM 781210079 (Authentication failure on persistent AVD with Conditional Access) | ||
|
|
||
| ## Problem Statement | ||
|
|
||
| Microsoft.Data.SqlClient's `ActiveDirectoryIntegrated` and other Public Client Application (PCA) authentication flows do not pass device information when acquiring tokens. This causes failures on persistent Azure Virtual Desktop (AVD) devices when Conditional Access Policies require device compliance or MFA based on device state. | ||
|
|
||
| ### Root Cause | ||
|
|
||
| MSAL's `AcquireTokenByIntegratedWindowsAuth` does not pass device claims to the identity provider. The Windows Web Account Manager (WAM) broker passes device information (PRT, device compliance state) to Entra ID, satisfying Conditional Access policies. | ||
|
|
||
| ### MSAL PCA Compliance | ||
|
|
||
| Microsoft identity platform requires first-party applications using Public Client Applications to use WAM broker on Windows for compliance. This ensures: | ||
|
|
||
| - Device-based Conditional Access policies work correctly | ||
| - Primary Refresh Token (PRT) is leveraged for SSO | ||
| - Device compliance state is included in token requests | ||
|
|
||
| ## Design | ||
|
|
||
| ### Target Location | ||
|
|
||
| The `ActiveDirectoryAuthenticationProvider` is in `src/Microsoft.Data.SqlClient.Extensions/Azure/src/`. This package targets `net462;netstandard2.0`. | ||
|
|
||
| ### Platform Support Matrix | ||
|
|
||
| | Platform | WAM Broker | Fallback | | ||
| | ---------- | ----------- | ---------- | | ||
| | Windows (.NET Framework 4.6.2+) | ✅ Supported | IWA (legacy) | | ||
| | Windows (.NET 8.0+ via netstandard2.0) | ✅ Supported | System browser | | ||
| | Linux/macOS (.NET via netstandard2.0) | ❌ Not available | System browser / IWA | | ||
|
|
||
| ### Authentication Modes Covered | ||
|
|
||
| | Mode | WAM Broker Behavior | | ||
| | ------ | ------------------- | | ||
| | `ActiveDirectoryInteractive` | Uses WAM for interactive token acquisition on Windows | | ||
| | `ActiveDirectoryIntegrated` | Uses WAM broker to pass device claims (solves CAP issues) | | ||
| | `ActiveDirectoryDeviceCodeFlow` | Uses WAM for device code flow on Windows | | ||
| | `ActiveDirectoryPassword` | Uses WAM for username/password flow on Windows | | ||
|
cheenamalhotra marked this conversation as resolved.
Outdated
|
||
| | `ActiveDirectoryDefault` | No change (uses Azure.Identity DefaultAzureCredential) | | ||
| | `ActiveDirectoryManagedIdentity` | No change (server-side, no WAM needed) | | ||
| | `ActiveDirectoryServicePrincipal` | No change (confidential client, no WAM needed) | | ||
| | `ActiveDirectoryWorkloadIdentity` | No change (workload identity, no WAM needed) | | ||
|
|
||
| ### Architecture Changes | ||
|
|
||
| 1. **Make class `partial`**: Split `ActiveDirectoryAuthenticationProvider` into platform-specific files | ||
| 2. **Add WAM broker**: Configure `BrokerOptions` on `PublicClientApplicationBuilder` on Windows | ||
| 3. **Parent window handle**: Provide window handle for WAM dialog (required by WAM on Windows) | ||
| 4. **Cross-platform `SetParentActivityOrWindow`**: Replace `#if NETFRAMEWORK`-only `SetIWin32WindowFunc` with cross-platform `Func<object>` API | ||
|
|
||
|
cheenamalhotra marked this conversation as resolved.
Outdated
|
||
| ### New Public APIs | ||
|
|
||
| ```csharp | ||
| public sealed partial class ActiveDirectoryAuthenticationProvider : SqlAuthenticationProvider | ||
| { | ||
| // Cross-platform API to set the parent window/activity for WAM dialog | ||
| // On Windows: accepts IntPtr (window handle) or IWin32Window via Func<object> | ||
| // On Unix: no-op (WAM not available) | ||
| public void SetParentActivityOrWindow(Func<object> parentActivityOrWindowFunc); | ||
|
cheenamalhotra marked this conversation as resolved.
Outdated
|
||
| } | ||
| ``` | ||
|
|
||
| ### Dependencies | ||
|
|
||
| - **New**: `Microsoft.Identity.Client.Broker` (same version as `Microsoft.Identity.Client`: 4.83.0) | ||
| - Conditional on Windows platform at runtime (the package includes platform-specific native binaries) | ||
|
|
||
| ### File Changes | ||
|
|
||
| | File | Change | | ||
| | ------ | -------- | | ||
| | `Directory.Packages.props` | Add `Microsoft.Identity.Client.Broker` version | | ||
| | `Azure.csproj` | Add package reference | | ||
| | `ActiveDirectoryAuthenticationProvider.cs` | Make partial, add broker logic | | ||
| | `ActiveDirectoryAuthenticationProvider.Windows.cs` (NEW) | Windows-specific: parent window detection | | ||
| | `Interop/Interop.GetConsoleWindow.cs` (NEW) | P/Invoke for kernel32 GetConsoleWindow | | ||
| | `Interop/Interop.GetAncestor.cs` (NEW) | P/Invoke for user32 GetAncestor | | ||
|
|
||
| ### Conditional Compilation Strategy | ||
|
|
||
| Since the Extensions/Azure project targets `net462;netstandard2.0`, we cannot use `#if _WINDOWS` (that's for the main SqlClient project). Instead: | ||
|
|
||
| - Use **runtime OS detection** (`RuntimeInformation.IsOSPlatform(OSPlatform.Windows)`) for broker activation | ||
| - The `Microsoft.Identity.Client.Broker` package is always referenced but only invoked on Windows | ||
| - Platform-specific partial class files use `#if NETFRAMEWORK` for .NET Framework-only code paths | ||
|
|
||
| ### Implementation Flow | ||
|
|
||
| ```flowchart | ||
| AcquireTokenAsync | ||
| ├── Non-PCA methods (Default, MSI, ServicePrincipal, Workload) → unchanged | ||
| └── PCA methods (Interactive, Integrated, Password, DeviceCodeFlow) | ||
| ├── Build PublicClientApplication with BrokerOptions (Windows only) | ||
| ├── Set ParentActivityOrWindow for WAM dialog | ||
| ├── Try silent token acquisition | ||
| └── If silent fails: | ||
| ├── Windows + Broker: WAM handles interactive/integrated flow | ||
| └── Non-Windows: Fallback to existing behavior (system browser, IWA) | ||
| ``` | ||
|
|
||
| ## Testing | ||
|
|
||
| ### Unit Tests | ||
|
|
||
| - Verify `SetParentActivityOrWindow` stores the function correctly | ||
| - Verify `SetParentActivityOrWindow` throws `ArgumentNullException` for null argument | ||
| - Verify `IsSupported` returns true for all expected auth methods | ||
|
|
||
| ### Manual/Integration Tests (require SQL Server) | ||
|
|
||
| - `ActiveDirectoryInteractive` with WAM on Windows | ||
| - `ActiveDirectoryIntegrated` with WAM on Windows (validates device claims pass) | ||
| - Verify Unix/macOS falls back to non-broker behavior | ||
| - Verify CAP-protected Azure SQL MI access works from AVD | ||
|
|
||
| ## Rollout | ||
|
|
||
| - WAM broker is **always enabled** on Windows when using PCA flows | ||
| - No opt-in connection string keyword needed (aligns with MSAL PCA compliance requirements) | ||
| - Existing `SetIWin32WindowFunc` remains as a backward-compatible API on .NET Framework, delegating to `SetParentActivityOrWindow` | ||
|
cheenamalhotra marked this conversation as resolved.
Outdated
|
||
59 changes: 59 additions & 0 deletions
59
...soft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.Windows.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System.Runtime.InteropServices; | ||
|
|
||
| namespace Microsoft.Data.SqlClient; | ||
|
|
||
| public sealed partial class ActiveDirectoryAuthenticationProvider | ||
|
paulmedynski marked this conversation as resolved.
|
||
| { | ||
| /// <summary> | ||
| /// Gets the parent window handle to be used for interactive authentication prompts | ||
| /// via the Windows Account Manager (WAM) broker. | ||
| /// </summary> | ||
| /// <returns> | ||
| /// The parent window handle as an <see cref="IntPtr"/>, or <see cref="IntPtr.Zero"/> if | ||
| /// not running on Windows or no window handle is available. | ||
| /// </returns> | ||
| private IntPtr GetParentWindow() | ||
| { | ||
| if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) | ||
| { | ||
| return IntPtr.Zero; | ||
| } | ||
|
|
||
| // If the user has provided a custom parent activity/window function, use it. | ||
| if (_parentActivityOrWindowFunc is not null) | ||
| { | ||
| object parentWindow = _parentActivityOrWindowFunc(); | ||
|
paulmedynski marked this conversation as resolved.
|
||
| if (parentWindow is IntPtr hwnd) | ||
| { | ||
| return hwnd; | ||
| } | ||
|
cheenamalhotra marked this conversation as resolved.
|
||
| } | ||
|
cheenamalhotra marked this conversation as resolved.
Outdated
|
||
|
|
||
| // Fall back to finding the console window, then getting its root owner. | ||
| IntPtr consoleHandle = Interop.Kernel32.GetConsoleWindow(); | ||
|
paulmedynski marked this conversation as resolved.
|
||
| if (consoleHandle != IntPtr.Zero) | ||
| { | ||
| IntPtr rootOwner = Interop.User32.GetRootOwner(consoleHandle); | ||
| if (rootOwner != IntPtr.Zero) | ||
| { | ||
| return rootOwner; | ||
| } | ||
| return consoleHandle; | ||
| } | ||
|
|
||
| return IntPtr.Zero; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the parent activity or window object for the broker authentication flow. | ||
| /// On Windows, returns the window handle. On other platforms, returns <see cref="IntPtr.Zero"/>. | ||
| /// </summary> | ||
| private object GetBrokerParentWindow() | ||
|
paulmedynski marked this conversation as resolved.
Outdated
|
||
| { | ||
| return GetParentWindow(); | ||
| } | ||
| } | ||
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
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
29 changes: 29 additions & 0 deletions
29
src/Microsoft.Data.SqlClient.Extensions/Azure/src/Interop/Interop.GetAncestor.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System.Runtime.InteropServices; | ||
|
|
||
| namespace Microsoft.Data.SqlClient; | ||
|
|
||
| internal static partial class Interop | ||
|
paulmedynski marked this conversation as resolved.
paulmedynski marked this conversation as resolved.
|
||
| { | ||
| internal static partial class User32 | ||
|
paulmedynski marked this conversation as resolved.
|
||
| { | ||
| private const uint GA_ROOTOWNER = 3; | ||
|
|
||
| /// <summary> | ||
| /// Retrieves the handle to the ancestor of the specified window. | ||
| /// </summary> | ||
| [DllImport("user32.dll")] | ||
| private static extern IntPtr GetAncestor(IntPtr hwnd, uint gaFlags); | ||
|
|
||
| /// <summary> | ||
| /// Gets the root owner window of the specified window handle. | ||
| /// </summary> | ||
| internal static IntPtr GetRootOwner(IntPtr hwnd) | ||
| { | ||
| return GetAncestor(hwnd, GA_ROOTOWNER); | ||
| } | ||
| } | ||
| } | ||
19 changes: 19 additions & 0 deletions
19
src/Microsoft.Data.SqlClient.Extensions/Azure/src/Interop/Interop.GetConsoleWindow.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System.Runtime.InteropServices; | ||
|
|
||
| namespace Microsoft.Data.SqlClient; | ||
|
|
||
| internal static partial class Interop | ||
|
paulmedynski marked this conversation as resolved.
|
||
| { | ||
| internal static partial class Kernel32 | ||
| { | ||
| /// <summary> | ||
| /// Retrieves the window handle used by the console associated with the calling process. | ||
| /// </summary> | ||
| [DllImport("kernel32.dll")] | ||
| internal static extern IntPtr GetConsoleWindow(); | ||
| } | ||
| } | ||
31 changes: 31 additions & 0 deletions
31
src/Microsoft.Data.SqlClient.Extensions/Azure/test/WamBrokerTests.cs
|
cheenamalhotra marked this conversation as resolved.
|
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| namespace Microsoft.Data.SqlClient.Extensions.Azure.Test; | ||
|
|
||
| public class WamBrokerTests | ||
| { | ||
| [Fact] | ||
| public void SetParentActivityOrWindow_NullArgument_ThrowsArgumentNullException() | ||
|
paulmedynski marked this conversation as resolved.
Outdated
|
||
| { | ||
| var provider = new ActiveDirectoryAuthenticationProvider(); | ||
| Assert.Throws<ArgumentNullException>("parentActivityOrWindowFunc", | ||
| () => provider.SetParentActivityOrWindow(null!)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void SetParentActivityOrWindow_ValidFunc_DoesNotThrow() | ||
| { | ||
| var provider = new ActiveDirectoryAuthenticationProvider(); | ||
| provider.SetParentActivityOrWindow(() => IntPtr.Zero); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void SetParentActivityOrWindow_CanBeCalledMultipleTimes() | ||
| { | ||
| var provider = new ActiveDirectoryAuthenticationProvider(); | ||
| provider.SetParentActivityOrWindow(() => IntPtr.Zero); | ||
| provider.SetParentActivityOrWindow(() => new IntPtr(12345)); | ||
|
cheenamalhotra marked this conversation as resolved.
Outdated
|
||
| } | ||
| } | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.