Skip to content
Closed

WAM POC #2884

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
<Compile Include="..\..\ref\Microsoft.Data.SqlClient.Batch.NetCoreApp.cs" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Identity.Client.Broker" Condition="$(DefineConstants.Contains('INTERACTIVE_AUTH'))">
<Version>$(MicrosoftIdentityClientBrokerVersion)</Version>
</PackageReference>
<PackageReference Include="Azure.Core" />
<PackageReference Include="Azure.Identity" />
<PackageReference Include="Microsoft.Bcl.Cryptography" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.Extensions.Caching.Memory" />
<PackageReference Include="Microsoft.Identity.Client.Broker" Condition="$(DefineConstants.Contains('INTERACTIVE_AUTH'))">
<Version>$(MicrosoftIdentityClientBrokerVersion)</Version>
</PackageReference>
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" />
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" />
<PackageReference Include="System.Buffers" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Debug|AnyCPU'" />
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Release|AnyCPU'" />
<PropertyGroup Condition="$(Configuration.Contains('Debug'))">
<DefineConstants>$(DefineConstants);DEBUG;DBG;_DEBUG;_LOGGING;RESOURCE_ANNOTATION_WORK;</DefineConstants>
<DefineConstants>$(DefineConstants);DEBUG;DBG;_DEBUG;_LOGGING;RESOURCE_ANNOTATION_WORK;INTERACTIVE_AUTH;</DefineConstants>
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The INTERACTIVE_AUTH constant is unconditionally defined in Debug configurations (line 76) but conditionally defined based on InteractiveAuthEnabled property in line 89. This creates a conflict where INTERACTIVE_AUTH will always be defined in Debug builds regardless of the InteractiveAuthEnabled setting. The unconditional definition on line 76 should be removed to respect the InteractiveAuthEnabled property setting.

Suggested change
<DefineConstants>$(DefineConstants);DEBUG;DBG;_DEBUG;_LOGGING;RESOURCE_ANNOTATION_WORK;INTERACTIVE_AUTH;</DefineConstants>
<DefineConstants>$(DefineConstants);DEBUG;DBG;_DEBUG;_LOGGING;RESOURCE_ANNOTATION_WORK;</DefineConstants>

Copilot uses AI. Check for mistakes.
<DebugType>Full</DebugType>
<Optimize>False</Optimize>
</PropertyGroup>
Expand All @@ -82,6 +82,12 @@
<DebugType>Pdbonly</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<PropertyGroup>
<DefineConstants>$(DefineConstants);NETFRAMEWORK;</DefineConstants>
</PropertyGroup>
<PropertyGroup Condition="'$(InteractiveAuthEnabled)'=='true'">
<DefineConstants>$(DefineConstants);INTERACTIVE_AUTH;</DefineConstants>
</PropertyGroup>
<ItemGroup>
<Reference Include="System" />
<Reference Include="System.Configuration" />
Expand Down Expand Up @@ -1069,6 +1075,21 @@
<PrivateAssets>All</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Azure.Identity">
<Version>$(AzureIdentityVersion)</Version>
</PackageReference>
<PackageReference Include="Microsoft.Identity.Client.Broker" Condition="$(DefineConstants.Contains('INTERACTIVE_AUTH'))">
<Version>$(MicrosoftIdentityClientBrokerVersion)</Version>
</PackageReference>
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect">
<version>$(MicrosoftIdentityModelProtocolsOpenIdConnectVersion)</version>
</PackageReference>
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens">
<version>$(MicrosoftIdentityModelJsonWebTokensVersion)</version>
</PackageReference>
<PackageReference Include="System.Buffers">
<Version>$(SystemBuffersVersion)</Version>
</PackageReference>
Comment on lines +1078 to +1092
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Duplicate package references detected. The following packages are referenced twice with different version specifications:

  • Azure.Identity (lines 1072 and 1078-1080)
  • Microsoft.IdentityModel.JsonWebTokens (lines 1087-1089 and 1094)
  • Microsoft.IdentityModel.Protocols.OpenIdConnect (lines 1084-1086 and 1095)
  • System.Buffers (lines 1090-1092 and 1096)

This will cause build issues. The newly added package references with explicit versions (lines 1078-1092) should be removed as these packages are already referenced earlier in the same ItemGroup without explicit versions.

Suggested change
<PackageReference Include="Azure.Identity">
<Version>$(AzureIdentityVersion)</Version>
</PackageReference>
<PackageReference Include="Microsoft.Identity.Client.Broker" Condition="$(DefineConstants.Contains('INTERACTIVE_AUTH'))">
<Version>$(MicrosoftIdentityClientBrokerVersion)</Version>
</PackageReference>
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect">
<version>$(MicrosoftIdentityModelProtocolsOpenIdConnectVersion)</version>
</PackageReference>
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens">
<version>$(MicrosoftIdentityModelJsonWebTokensVersion)</version>
</PackageReference>
<PackageReference Include="System.Buffers">
<Version>$(SystemBuffersVersion)</Version>
</PackageReference>
<PackageReference Include="Microsoft.Identity.Client.Broker" Condition="$(DefineConstants.Contains('INTERACTIVE_AUTH'))">
<Version>$(MicrosoftIdentityClientBrokerVersion)</Version>
</PackageReference>

Copilot uses AI. Check for mistakes.
<PackageReference Include="Microsoft.Extensions.Caching.Memory" />
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" />
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using System;
using System.Runtime.InteropServices;

internal partial class Interop
{
internal partial class Kernel32
{
[DllImport("kernel32.dll")]
internal static extern IntPtr GetConsoleWindow();
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Minimise the use of unmanaged code.

Copilot uses AI. Check for mistakes.
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
using System;
using System.Runtime.InteropServices;


internal partial class Interop
{
internal partial class User32
{
internal enum GetAncestorFlags
{
GetParent = 1,
GetRoot = 2,
/// <summary>
/// Retrieves the owned root window by walking the chain of parent and owner windows returned by GetParent.
/// </summary>
GetRootOwner = 3
}

/// <summary>
/// Retrieves the handle to the ancestor of the specified window.
/// </summary>
/// <param name="hwnd">A handle to the window whose ancestor is to be retrieved.
/// If this parameter is the desktop window, the function returns NULL. </param>
/// <param name="flags">The ancestor to be retrieved.</param>
/// <returns>The return value is the handle to the ancestor window.</returns>
[DllImport("user32.dll", ExactSpelling = true)]
internal static extern IntPtr GetAncestor(IntPtr hwnd, GetAncestorFlags flags);
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

Minimise the use of unmanaged code.

Copilot uses AI. Check for mistakes.
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using System;
using System.Threading;
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The using directive for System.Threading on line 2 is unused in this file. The file only declares a property and doesn't use any threading-related types. This should be removed.

Suggested change
using System.Threading;

Copilot uses AI. Check for mistakes.

namespace Microsoft.Data.SqlClient
{
public sealed partial class ActiveDirectoryAuthenticationProvider : SqlAuthenticationProvider
{
private Func<object> _parentActivityOrWindowFunc = null;

Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The Unix implementation returns null when _parentActivityOrWindowFunc is null, but there's no corresponding SetParentActivityOrWindow method in the Unix partial class. This creates an API inconsistency between Windows and Unix platforms. Users who call SetParentActivityOrWindow on Unix will encounter a compile error, but the property ParentActivityOrWindow will always return null, which may cause issues when interactive authentication is attempted on Unix (if supported). Consider either adding a stub implementation or documenting the platform-specific behavior.

Suggested change
public void SetParentActivityOrWindow(Func<object> parentActivityOrWindowFunc)
{
_parentActivityOrWindowFunc = parentActivityOrWindowFunc;
}

Copilot uses AI. Check for mistakes.
private Func<object> ParentActivityOrWindow => _parentActivityOrWindowFunc;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
using System;

namespace Microsoft.Data.SqlClient
{
public sealed partial class ActiveDirectoryAuthenticationProvider : SqlAuthenticationProvider
{
private Func<object> _parentActivityOrWindowFunc = null;

private Func<object> ParentActivityOrWindow
{
get
{
return _parentActivityOrWindowFunc != null ? _parentActivityOrWindowFunc : GetConsoleOrTerminalWindow;
}

set
{
_parentActivityOrWindowFunc = value;
}
}

#if NETFRAMEWORK
/// <include file='../../../../../../doc/snippets/Microsoft.Data.SqlClient/ActiveDirectoryAuthenticationProvider.xml' path='docs/members[@name="ActiveDirectoryAuthenticationProvider"]/SetIWin32WindowFunc/*'/>
public void SetIWin32WindowFunc(Func<System.Windows.Forms.IWin32Window> iWin32WindowFunc) => SetParentActivityOrWindow(iWin32WindowFunc);
#endif

/// <summary>
///
/// </summary>
/// <param name="parentActivityOrWindowFunc"></param>
Comment on lines +28 to +30
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The SetParentActivityOrWindow method (line 31) lacks XML documentation. Since this is a new public API method, it should include proper XML documentation similar to other public methods in this class, describing its purpose, parameters, and usage.

Suggested change
///
/// </summary>
/// <param name="parentActivityOrWindowFunc"></param>
/// Sets the delegate used to obtain the parent activity or window for interactive authentication prompts.
/// </summary>
/// <param name="parentActivityOrWindowFunc">
/// A function that returns an object representing the parent activity or window handle to associate with UI dialogs.
/// </param>

Copilot uses AI. Check for mistakes.
public void SetParentActivityOrWindow(Func<object> parentActivityOrWindowFunc) => this.ParentActivityOrWindow = parentActivityOrWindowFunc;

private object GetConsoleOrTerminalWindow()
{
IntPtr consoleHandle = Interop.Kernel32.GetConsoleWindow();
IntPtr handle = Interop.User32.GetAncestor(consoleHandle, Interop.User32.GetAncestorFlags.GetRootOwner);

return handle;
}
}
}
Loading
Loading