dotnet / sign

Code Signing CLI tool supporting Authenticode, NuGet, VSIX, and ClickOnce
MIT License
439 stars 84 forks source link

CLI: simplify Azure authentication #724

Open dtivel opened 1 week ago

dtivel commented 1 week ago

This thread generated some ideas on how we can improve the CLI user experience for Azure authentication, which is currently used by Azure Key Vault signing. In the near future, it will also be used by Trusted Signing signing.

Background

Sign CLI's Azure Key Vault signing exposes 4 authentication-specific options. The original intent was to support 2 authentication strategies:

Problem statement

Managed identity support is provided through Azure SDK for .NET's DefaultAzureCredential class. This class actually supports 10 authentication strategies, which are tried in a fixed order until one succeeds. In most cases, this may work like magic. However, there are several problems with this solution.

  1. --azure-key-vault-managed-identity doesn't actually guarantee we'll use the managed identity authentication strategy. Authentication can fail because DefaultAzureCredential succeeded in obtaining a token with an earlier (unwanted) authentication strategy. This really happens.
  2. There's no clear support for other authentication strategies (like Azure CLI). You could use the --azure-key-vault-managed-identity option, set a bunch of ExcludeXXXCredential options for all the options before Azure CLI authentication strategy, but that's a hacky, terrible user experience.
  3. From a CLI UX perspective, to formally support other authentication strategies (like Azure CLI), we'd need to expose an option for it and validate the various combinations of options.

Proposed solution

First, deprecate the 4 authentication-specific options. Continue to honor the options but output a warning that users should migrate to the new solution, which is one of the following options:

At some time in the future, remove the 4 authentication-specific options. By default, Azure Key Vault signing will require no Azure authentication options. It will support all authentication strategies provided by DefaultAzureCredential in the order that class provides.

For users who want to use a specific authentication strategy, a new option (-act, --azure-credential-type) will enable that. Note that this list of accepted values represents only a subset of strategies provided by DefaultAzureCredential on the principal that we'd add others when needed.

Value Strategy
environment EnvironmentCredential
managed-identity ManagedIdentityCredential

CC @clairernovotny, @joelverhagen, @dlemstra

dlemstra commented 1 week ago

I think we should also make the "old" TenantIdOption obsolete and introduce a new option called -ati, --azure-tenant-id that sets the TenantId property of the DefaultAzureCredentialOptions. The --azure-key-vault-tenant-id option should print a warning when it is being used.

And when we add managed-identity we should probably also add options to set the ManagedIdentityClientId and ManagedIdentityResourceId properties of the DefaultAzureCredentialOptions.

dlemstra commented 4 days ago

I think we should also add azure-cli as an option. I am using az login in my pipeline and I would like to only have the credential type enabled.

clairernovotny commented 3 days ago

We discussed this a bit offline and for now decided to go with using DefaultAzureCredential as the default authentication mechanism and not to have an additional credential type option. We will document that authentication happens using that mechanism and defer to the Azure docs on which options are evaluated in what order.

While an issue exists with DefaultAzureCredential in one case, we don't yet know the root cause, if it's specific to the JavaScript version, something with AzDo, or something else entirely. Once we have a root cause, we can evaluate the best way to ensure it doesn't happen here.