[Bug] Suggest to use WAM/Broker where applicable

AzureAD / microsoft-authentication-library-for-dotnet

Microsoft Authentication Library (MSAL) for .NET

https://aka.ms/msal-net

MIT License

1.39k stars 341 forks source link

[Bug] Suggest to use WAM/Broker where applicable #3988

Open gladjohn opened 1 year ago

gladjohn commented 1 year ago

Logs and network traces 2023-03-02 14:10:13.810 -08:00 [INF] Failed to retrieve the Proxy App authentication token MSAL.Desktop.4.47.2.0.MsalClientException: ErrorCode: integrated_windows_auth_not_supported_managed_user Microsoft.Identity.Client.MsalClientException: Integrated Windows Auth is not supported for managed users. See https://aka.ms/msal-net-iwa for details. at Microsoft.Identity.Client.Internal.Requests.IntegratedWindowsAuthRequest.d__5.MoveNext()

Which version of MSAL.NET are you using? Latest

Platform Net Core

What authentication flow has the issue?

Desktop / Mobile
- [ ] Interactive
- [x] Integrated Windows Authentication
- [ ] Username Password
- [ ] Device code flow (browserless)
Web app
- [ ] Authorization code
- [ ] On-Behalf-Of
Daemon app
- [ ] Service to Service calls

Other?

Is this a new or existing app? Existing

Improved messaging / supportability Suggest to use WAM/Broker where applicable

bgavrilMS commented 1 year ago

Agreed. I would do the folloiwng:

~- [ ] This exception should be MsalUIRequiredException to minimize impact~

[ ] Soft deprecate IWA
[ ] Remove IWA sample (or replace it with WAM broker)
[ ] Remove all samples that do not use WAM on windows.

pmaytak commented 1 year ago

nit:

This exception should be MsalUIRequiredException to minimize impact.

Breaking change - changing the exception type?

Remove IWA sample (or replace it with WAM broker) Remove all samples that do not use WAM on windows.

Keep them and archive, don't remove. May be helpful when debugging / investigating current issues or when someone can't use a broker yet for whatever reason.

bgavrilMS commented 1 year ago

@pmaytak - MsalUiReaquiredException is a subtype of MsalServiceException so this is not a breaking change.

bgavrilMS commented 1 year ago

Would like @localden 's and @alextok opinion on this subject, i.e. are we ok to [soft / hard] deprecate IWA from MSALs once broker auth is GA-ed?

pmaytak commented 1 year ago

@pmaytak - MsalUiReaquiredException is a subtype of MsalServiceException so this is not a breaking change.

We currently throw MsalClientException, in this case.

bgavrilMS commented 1 year ago

Yes, you are right :(. That's unfortunate.

localden commented 1 year ago

@bgavrilMS do we log in any capacity today IWA usage? In typical PM fashion would like to get some data on who might depend on that flow today. Theoretically, IWA serves the same purpose as the broker, no? Are all the versions of Windows that we support today with MSAL.NET have the broker ready?

bgavrilMS commented 1 year ago

@localden

Are all the versions of Windows that we support today with MSAL.NET have the broker ready? - no, the broker will not work on Windows Server 2012 and 2016, but should work on all Windows non-server versions, since Microsoft supports only 10 and 11 now. So yeah, that's a good point.
There is a lot of usage of this API indeed (I sent you a query via email and some results).
IWA serves the same purpose as the broker - on Windows, both IWA and the broker can be used to sign-in the current user silently. The broker has a few mechanisms of doing this, including IWA, so there's a much better chance that it'll work.