[Bug] Microsoft.Identity.Client references System.Windows.Forms

[pedrolupin]

Library version used

4.56.0

.NET version

.net 7.0.14

Scenario

ManagedIdentityClient - managed identity

Is this a new or an existing app?

The app is in production, and I have upgraded to a new version of MSAL

Issue description and reproduction steps

Azure.Identity 1.9.0 -> Microsoft.Identity.Client 4.49.1 did not have a dependency on System.Windows.Forms, but when updating to newer Azure.Identity 1.10.4 -> Microsoft.Identity.Client 4.56.0 the dependency to Windows.Forms appeared, this is a problem.

Azure.Identity should be able to run headless, on servers and many situations that don't have access to System.Windows.Forms because the .NET desktop runtime is not present.

Relevant code snippets

No response

Expected behavior

No response

Identity provider

Azure B2C Basic Policy

Regression

4.49.1

Solution and workarounds

No response

[bgavrilMS]

What's the actual problem @pedrolupin ? The dependency will not be used in headless scenarios.

Also, if you target pure .NET, the dependency is not present. MSAL.NET always had a dependency on Windows.Forms on .NET Fwk (not Core).

[pedrolupin]

The problem is that in the web server running IIS we don't have .net desktop runtime because it is not needed.

So the update made the web app stop working.

The project is compiled for .net 7.0 pure.

El mié, 13 dic 2023 17:11, Bogdan Gavril @.***> escribió:

What's the actual problem @pedrolupin https://github.com/pedrolupin ? The dependency will not be used in headless scenarios.

Also, if you target pure .NET, the dependency is not present.

— Reply to this email directly, view it on GitHub https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/4468#issuecomment-1854231988, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABA7CDG4XEJ65EGDPAMFMXLYJHHSZAVCNFSM6AAAAABATKNZOKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJUGIZTCOJYHA . You are receiving this because you were mentioned.Message ID: <AzureAD/microsoft-authentication-library-for-dotnet/issues/4468/1854231988 @github.com>

[pmaytak]

Microsoft.Identity.Client adds Windows Forms reference only on net6.0-windows and net462 platforms. So net7.0 app should pick up net6.0 MSAL binary without the Forms. What's the exception that you get?

[pedrolupin]

I dug a bit more to see what happens here.

We have a project that accesses Azure Keyvault and hence requires Azure.Identity -> Microsoft.Identity.Client... This project uses net7.0 as the target framework. But the startup project that references our keyvault project is an ASP.NET Core application that uses net7.0-windows as the target framework. It has to use the Windows OS target since it uses APIs only available for that target.

Before the update of Azure.Identity from 1.9.0 to 1.10.4, there was no reference to System.Windows.Forms.

Just because the target is net7.0-windows that doesn't necessarily imply that it's a desktop application. This must be a common usage for ASP.NET users that want to use Azure Identity... and the IIS servers use ASP.NET Core Shared Framework , .NET Runtime and Windows Server Hosting packages, but there was no dependency to the .NET Desktop Runtime and it makes no sense to require it in a web server.

Thanks for looking into this, I appreciate the effort to answer and hopefully solve our use case.

Regards, Pedro

On Wed, 13 Dec 2023 at 23:30, Peter @.***> wrote:

Microsoft.Identity.Client adds Windows Forms reference only on net6.0-windows and net462 platforms. So net7.0 app should pick up net6.0 MSAL binary without the Forms. What's the exception that you get?

— Reply to this email directly, view it on GitHub https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/4468#issuecomment-1854798914, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABA7CDHGMLAZGBPCO7KWB6TYJIT7TAVCNFSM6AAAAABATKNZOKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJUG44TQOJRGQ . You are receiving this because you were mentioned.Message ID: <AzureAD/microsoft-authentication-library-for-dotnet/issues/4468/1854798914 @github.com>

[pmaytak]

Just because the target is net7.0-windows that doesn't necessarily imply that it's a desktop application.

Indeed. This is a known gap. We don't really have a good solution at the moment, but exploring some options. Some of the workarounds are to deploy the app as self-contained or have a post-build script which removes the Windows Forms reference from the runtime config JSON.

[SimonCropp]

@pmaytak that this bit us this week. updated to a the security hotfix patch for sql client nuget. and our web apps would no longer run

[SimonCropp]

given this prevents apps from starting. perhaps it should be a p1?

[bgavrilMS]

Thanks @SimonCropp , we will discuss this in the next few days and make a decisions. Bumping the major version of MSAL from version 4 (it's been this way for 4+ years) to version 5 is really big deal and we are not ready for it.

We are evaluating if we can introduce this change in a non-breaking way (well, similar to [Obsolete]-ing some API at least), maybe with some decent msbuild warning.

[SimonCropp]

@bgavrilMS i dont see a way of doing this in a non-breaking way. even if we have the API as Obsolete with warning, it means it still needs to work if people ignore the warning. WindowsFormsWebAuthenticationDialog is exposed as a public type that inherits from System.Windows.Forms.Form. so that means System.Windows.Forms must continue to be referenced

IMO we should bite the bullet and make a breaking change to fix this. it would be relatively low impact in terms of scale for a breaking change. the winforms bit could be moved to a diff nuget and people add a reference to that nuget.

[nblumhardt]

We were able to work around it in our usage by abandoning net8.0-windows and reorganizing our project to publish for only net8.0 instead. I'm not sure if/where it will bite us further down the track :-) but just post this in case it helps someone else out in the meantime.

[robertmuehsig]

We encountered the same problem and were also forced to "migrate" from "net6.0-windows" to "net6.0". Of course, this was only possible because we "muted" this rule in .editorconfig at the same time: dotnet_diagnostic.CA1416.severity = none

We use a number of Windows-specific APIs (e.g. DirectorySearcher) - that was also the purpose of the net6.0-windows TargetFramework. Of course, we don't know if any other NuGet package might have a different behavior because we are no longer following the Windows-specific case (I'm not sure what "switches" you have in NuGet packages to create a different behavior based on the target systems)

The whole problem is quite complex. We had to update Azure.Identity because the older version contained a security vulnerability and Visual Studio "forced" the update on us (which is ok). Since all runtimes are installed on our dev environments, we were a bit surprised that the application did not run on Azure - especially with such a "minor update", we would not have assumed a breaking change.

We use Azure.Identity for DataProtection, which is even recommended as best practice by Microsoft.

Basically, this change made the "-windows" TargetFrameworks unusable and I'm surprised that this didn't make bigger waves. We are a small development team and can, for example, easily change rules in .editorconfig or customize the TargetFrameworks.

The idea that -windows means it runs on the desktop is also simply a complete misinterpretation.

The fact that Visual Studio now encourages everyone to update Azure.Identity and thus also includes this dependency here will lead to a flood of these bugs.

Sorry for the long comment, but this bugged me.

[bjornbouetsmith]

We were able to work around it in our usage by abandoning net8.0-windows and reorganizing our project to publish for only net8.0 instead. I'm not sure if/where it will bite us further down the track :-) but just post this in case it helps someone else out in the meantime.

Thats a poor solution in my opinion - also the work around we made made ourselves - having a post-build script that removes the Desktop reference from the runtime config.

Whoever added Windows.Forms as a dependency to this library must have red ears - and in my opinion that change should be either rolled back, or a new version released immediately with this dependency removed.

[SimonCropp]

i also want winforms (and related dlls/pdbs) gone so they dont bloat my app deployments

[localden]

Hey folks - PM for Auth SDKs at Microsoft here. Appreciate you bringing up the issue to our attention, I 100% agree that Windows Forms components in cases where no UI components are used directly is less than optimal. @bgavrilMS, @pmaytak, @gladjohn and team have met earlier last week to discuss our options and are currently assessing what the best path forward is. One alternative that we're considering is rev-ing the version of MSAL and introducing a change where UI components are moved into another package (e.g., Microsoft.Identity.Client.Desktop) that can be used whenever desktop-related flows need to be in place (e.g., WebView2-based interactive prompts).

We are still working through the potential designs and will follow-up on this thread in the next few weeks with more details. Again - I reallyt appreciate the fact that the community is vocal about the issue. We're looking at the best solution as we speak.

[bjornbouetsmith]

introducing a change where UI components are moved into another package (e.g., Microsoft.Identity.Client.Desktop) that can be used whenever desktop-related flows need to be in place (e.g., WebView2-based interactive prompts).

I think that is the only sane approach, although I would probably call the package something with UIthen its similar to e.g. System.Runtime.WindowsRuntime.UI.Xaml - although .WinUIseems to also be pupular as a suffix

[SimonCropp]

again i ask, given this prevents apps from starting, can this be made a p1?

[SimonCropp]

@localden it seems the webview and winforms code already exist in the existing Microsoft.Identity.Client.Desktop nuget. so all that is required is to remove the webview and winforms code from Microsoft.Identity.Client?

[bgavrilMS]

@SimonCropp - yeah, all we'd have to do is to stop building MSAL against net6-windows. Breaking changes are not really handled well in most languages, including C#. See what happened to Newtonsoft.

Also, I want to make it clear that we are exclusively talking about .NET here and not .NET FWK. We won't make changes to MSAL for .NET FWK, it's been dependening on WinForms for 5+ years.

[nicolascotton]

I am using Microsoft.Identity.Client in my Windows services, but they are now failing to start with the .NET runtime. Installing the desktop runtime is not an acceptable solution for me. Would it be possible to have a solution that is not reliant on these unnecessary components for my services?

[bgavrilMS]

Yes, we will drop support for net6-windows from MSAL. The extra logic lives in MSAL.Desktop. I do not have an ETA for this, but hope we'll get it done in a few weeks.

[rarowston]

@bgavrilMS Any updates/ETAs on this one? We're using workarounds for it for the time being, but it would be good to get things working fully again.

[alekdavis]

We just ran into this issue with an ASP.NET Core REST API app built for net8.0-windows. We need the -windows OS only because the app uses System.DirectoryServices (which would not build just for net8.0). So moving to a non-OS-specific build is not an option. What are the other workarounds now?

[rarowston]

We just ran into this issue with an ASP.NET Core REST API app built for net8.0-windows. We need the -windows OS only because the app uses System.DirectoryServices (which would not build just for net8.0). So moving to a non-OS-specific build is not an option. What are the other workarounds now?

Others might be able to chime in with other options, or clarification on how to do number 4, but there are 4 options that I have seen mentioned to work around this issue.

1. Remove the -windows option in the target framework (This is the option I am using as it is the simplest). I'm not sure what the issue you are having with this as I am also using DirectoryServices and it builds 'fine' after I suppress all the warnings that this causes.
2. Deploy the runtime to the target server. It is a bit odd to deploy the desktop runtime to a web server - but that's why we call it a workaround.
3. Downgrade the version of this package before the issue was introduced, though this re-introduces a potential security vulnerability - so I don't like this option much.
4. There has been mention above of using a post-build script to manually remove the dependency from the runtime config file though I am not sure how well this will work in practice (i.e. if it will ever attempt to access functions from this runtime in your usage) https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/4468#issuecomment-1857228932

[SimonCropp]

@alekdavis @rarowston

Remove the -windows option in the target framework (This is the option I am using as it is the simplest). I'm not sure what the issue you are having with this as I am also using DirectoryServices and it builds 'fine' after I suppress all the warnings that this causes.

thats the workaround we went with. had to suppress some warnings about "please use -windows". but at runtime it works

[alekdavis]

@rarowston @SimonCropp Ah, thank you soooo much. I did not realize that this was just a warning that can be safely ignored, so I disabled it, switched OS platform from Windows to (none) and it worked. In case anyone wonders, the easies way to disable the warning was via this line in the .editorconfig file:

dotnet_diagnostic.CA1416.severity = none

[SimonCropp]

@alekdavis yeah the warning can be interpreted as "if you deploy this to non windows, it wont work"

[rclabo]

I just got bit by this. Sucks. Any update on the status of the fix?

[pedrolupin]

This was added to the 4.60.0 Milestone · GitHub https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/milestone/124, then pushed to 4.61.0 Milestone · GitHub https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/milestone/127 which is due on May, 2. Fingers crossed :)

El mié, 17 abr 2024, 15:50, Ron Clabo @.***> escribió:

I just got bit by this. Sucks. Any update on the status of the fix?

— Reply to this email directly, view it on GitHub https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/4468#issuecomment-2061311216, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABA7CDCGSHV42QRLQIU6B5TY5Z4ZVAVCNFSM6AAAAABATKNZOKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRRGMYTCMRRGY . You are receiving this because you were mentioned.Message ID: <AzureAD/microsoft-authentication-library-for-dotnet/issues/4468/2061311216 @github.com>

[rclabo]

There has been mention above of using a post-build script to manually remove the dependency from the runtime config file though I am not sure how well this will work in practice (i.e. if it will ever attempt to access functions from this runtime in your usage) https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/4468#issuecomment-1857228932

It's been mentioned that one workaround is to remove the "Microsoft.WindowsDesktop.App" dependency from the runtimeconfig.json file. However this approach did not work for us.

Changing the file from

{
  "runtimeOptions": {
    "tfm": "net8.0",
    "frameworks": [
      {
        "name": "Microsoft.NETCore.App",
        "version": "8.0.0"
      },
      {
        "name": "Microsoft.WindowsDesktop.App",
        "version": "8.0.0"
      },
      {
        "name": "Microsoft.AspNetCore.App",
        "version": "8.0.0"
      }
    ],
    "configProperties": {
      "System.GC.Server": false,
      "System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization": false
    }
  }
}

to

{
  "runtimeOptions": {
    "tfm": "net8.0",
    "frameworks": [
      {
        "name": "Microsoft.NETCore.App",
        "version": "8.0.0"
      },
      {
        "name": "Microsoft.AspNetCore.App",
        "version": "8.0.0"
      }
    ],
    "configProperties": {
      "System.GC.Server": false,
      "System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization": false
    }
  }
}

for us did help to allow the website to start up but as soon as the code attempted to use Microsoft.Data.SqlClient the SqlClient throws with the following exception:

 ---> System.TypeInitializationException: The type initializer for 'Microsoft.Data.SqlClient.SqlAuthenticationProviderManager' threw an exception.
 ---> System.IO.FileNotFoundException: Could not load file or assembly 'System.Configuration.ConfigurationManager, Version=8.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'. The system cannot find the file specified.
File name: 'System.Configuration.ConfigurationManager, Version=8.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'
   at Microsoft.Data.SqlClient.SqlAuthenticationProviderManager..cctor()
   --- End of inner exception stack trace ---
   at Microsoft.Data.ProviderBase.DbConnectionPool.TryGetConnection(DbConnection owningObject, UInt32 waitForMultipleObjectsTimeout, Boolean allowCreate, Boolean onlyOneCheckConnection, DbConnectionOptions userOptions, DbConnectionInternal& connection)
   at Microsoft.Data.ProviderBase.DbConnectionPool.TryGetConnection(DbConnection owningObject, TaskCompletionSource`1 retry, DbConnectionOptions userOptions, DbConnectionInternal& connection)
   at Microsoft.Data.ProviderBase.DbConnectionFactory.TryGetConnection(DbConnection owningConnection, TaskCompletionSource`1 retry, DbConnectionOptions userOptions, DbConnectionInternal oldConnection, DbConnectionInternal& connection)
   at Microsoft.Data.ProviderBase.DbConnectionInternal.TryOpenConnectionInternal(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource`1 retry, DbConnectionOptions userOptions)
   at Microsoft.Data.ProviderBase.DbConnectionClosed.TryOpenConnection(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource`1 retry, DbConnectionOptions userOptions)
   at Microsoft.Data.SqlClient.SqlConnection.TryOpen(TaskCompletionSource`1 retry, SqlConnectionOverrides overrides)
   at Microsoft.Data.SqlClient.SqlConnection.Open(SqlConnectionOverrides overrides)
   at Microsoft.Data.SqlClient.SqlConnection.Open()
   at App.Sql.SqlTube.OpenConnection(SqlCommand command, Boolean makeMultipleTries) in C:\Users\RonC\source\repos\wwwGiftOasis3\MP_Prims\Sql\SqlTube.cs:line 332

Has anyone managed to make that workaround actually work?

[localden]

Hey folks - we're currently working on mitigating this. @bgavrilMS has been working on this in https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/4567

[rclabo]

@localden - Your work on this is very important to the community. Thanks you for the update.

I did manage to work around the issue by doing a self contained deployment of our website so at least we are up and running again. We look forward to getting back to non-self-contained deployments as usual once the bug is fixed.

Again, much appreciate the update and the fact that this is actively being worked on.

[pedrolupin]

Thanks Bogdan looking forward to the release :)

[bgavrilMS]

The team aims to release around the end of April.

[gautamdsheth]

@bgavrilMS - does this mean that the with 4.61.0 release, it will also work with net6.0-windows ? Or do we need to change anything in the application targeting that ? Sorry, didn't get it from the above thread.

[pmaytak]

Hello. We've released MSAL 4.61.0 which removes net6.0-windows binary (Windows Forms dependency).

For confidential client apps targeting net6.0-windows, there are no migration steps. You should be able to upgrade MSAL and remove the workarounds. The app will pick up net6.0 MSAL binary.

For public client apps targeting net6.0-windows, reference Microsoft.Identity.Client.Broker when using interactive authentication with Windows Broker and call WithBroker(BrokerOptions); or reference Microsoft.Identity.Client.Desktop when authenticating with browser and call WithWindowsEmbeddedBrowserSupport(). There are no changes to the usage of the system browser.

@pedrolupin @SimonCropp @nblumhardt @robertmuehsig @bjornbouetsmith @nicolascotton @alekdavis @rarowston @rclabo @gautamdsheth @Kavya-Gogula @ayrton97