Skip to content
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

Enable CanShowDialog for .NET core Authentication Plugins #14010

Open
embetten opened this issue Dec 19, 2024 · 5 comments
Open

Enable CanShowDialog for .NET core Authentication Plugins #14010

embetten opened this issue Dec 19, 2024 · 5 comments
Assignees
Labels
Category:Quality Week Issues that should be considered for quality week Partner:AzureDevOps Partner:DotNet Partner:MSBuild Priority:2 Issues for the current backlog. Product:dotnet.exe Type:DCR Design Change Request

Comments

@embetten
Copy link

embetten commented Dec 19, 2024

NuGet Product(s) Affected

MSBuild.exe, dotnet.exe

Current Behavior

Currently, NuGet does not enable canShowDialog for .NET core authentication plugins. In these scenarios, canShowDialog is hard-coded to false in the defaultCredentialService. As a result, users with a .NET core NuGet authentication plugin backed by MSAL will be forced down device code flow instead of interactive authentication flow even if a pop up or browser auth is available and preferred.

Desired Behavior

To help users avoid device code flow where possible, this behavior should be changed to allow the canShowDialog parameter to be passed by the user to .NET Core plugins. Additionally, the relevant dotnet cli NuGet commands and other .NET core integration points should be updated to pass the canShowDialog argument. This change will give users the flexibility to choose their preferred authentication method, enhancing both usability and security.

Additional Context

Additional context to consider:

  • MSAL is the standard authentication library. For users that do not have WAM/WIA broker authentication available, device code flow is the only authentication method available without a pop-up dialog.
  • Device code flow is a negative user experience compared to other MSAL authentication methods and a higher-risk authentication flow. Device code flow cannot be bound to a device, which prevents Entra tenant admins from adding conditional access policies that check for specific devices. These integration points should be updated to enable our security-conscious users to easily avoid device code flow where possible.
  • With mono no longer being supported, more users are being pushed to the .NET core plugins. Without this proposed behavior change, more users will be pushed to device code flow. Users migrating from the .NET framework experience to the .NET core behavior will no longer have canShowDialog option thereby degrading their experience.
  • The artifacts-credprovider is considering long term plans to return Entra tokens instead of ADO PAT tokens. In doing so, the login frequency would be reduced to the lifetime of the Entra tokens. Without this proposed behavior change, users would be prompted via device code flow every 24-hour period. We would like users to have an easier way to specify device code flow alternatives to make this migration a more viable user experience.
  • By popular demand, artifacts-credprovider has implemented a workaround to allow users to override this behavior by setting an environment variable NUGET_CREDENTIALPROVIDER_FORCE_CANSHOWDIALOG_TO (see here in the readme). However, we suspect this environment variable is not widely known, and setting environment variables instead of passing command line arguments is a disjointed experience for CLI users.
@nkolev92
Copy link
Member

It's not just .NET (Core), but MSBuild.exe as well.

@jeffkl
Copy link
Contributor

jeffkl commented Jan 6, 2025

Team Triage: We should discuss with .NET partners and determine if we can set canShowDialog to true if the user specified that the restore can be interactive.

@jeffkl jeffkl added Priority:2 Issues for the current backlog. Category:Quality Week Issues that should be considered for quality week labels Jan 6, 2025
@baronfel
Copy link

baronfel commented Jan 30, 2025

We chatted today and SDK and MSBuild are on-board. We see two parts to this work:

  • NuGet making the change to canShowDialog, so that users that do invoke interactive auth get a nicer experience
  • MSBuild.exe/dotnet getting a bit smarter and defaulting --interactive to true for user sessions that are interactive. We've built some muscle about how to do this thanks to Terminal Logger that we can try to reuse here.

We think these two things can proceed in parallel - NuGet could consider shipping this for 17.14, while msbuild.exe and dotnet's changes (which are potentially more breaking?) may wait until .NET 10.

The only thing we wanted to ensure was that after the change, the msbuild.exe console logger and the dotnet Terminal Logger both still recognize and print a message telling the user what's happening while the build is blocked waiting for the auth flow, in case the dialog loses focus or something.

embetten added a commit to microsoft/artifacts-credprovider that referenced this issue Feb 12, 2025
Adding message to better notify that the authentication is blocked and
waiting for user interaction.

![image](https://github.com/user-attachments/assets/51f29695-6189-444a-9de0-d897d6a9772d)

Coming in part for this ask:
NuGet/Home#14010 (comment)
@jgonz120
Copy link
Contributor

jgonz120 commented Feb 20, 2025

For the NuGet implementation we need to make sure that credential providers prepend [CredentialProvider], which msbuild uses to see if a message needs to be displayed right away.

LogRequestHandler can be updated to take in a message prefix and prepend it to any messages that are logged, if it isn't already present.

DefaultCredentialServiceUtility.GetCredentialProvidersAsync can be updated to always set canShowDialog to true.

@nkolev92
Copy link
Member

So we're assuming if --interactive is used, then canShowDialog is true

Those are unrelated.
They're already separate parameters that mean different things.
You're just changing the type of interactivity.

noninteractive is a CredentialService setting, can show dialog is something you tell the credential provider because it decides the type of interactivity.

https://github.com/NuGet/NuGet.Client/blob/ab6d3b886b32e9b8f68a7f1b299c43b0c72487dc/src/NuGet.Core/NuGet.Credentials/DefaultCredentialServiceUtility.cs?plain=1#L37

So just change canShowDialog to true and you're good.

@jgonz120 jgonz120 self-assigned this Feb 24, 2025
embetten added a commit to microsoft/artifacts-keyring that referenced this issue Mar 20, 2025
- CanShowDialog should be defaulted to true for the reasons outlined in NuGet/Home#14010
- If a system does not have the requirements to pop up a dialog (no browser support) the credprovider will fall through to device code flow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:Quality Week Issues that should be considered for quality week Partner:AzureDevOps Partner:DotNet Partner:MSBuild Priority:2 Issues for the current backlog. Product:dotnet.exe Type:DCR Design Change Request
Projects
None yet
Development

No branches or pull requests

5 participants