azure-sdk-for-net
azure-sdk-for-net copied to clipboard
[FEATURE REQ] Restore include options in DefaultAzureCredentialOptions.
Library name
Azure.Identity
Please describe the feature.
The current implementation of DefaultAzureCredentialOptions includes Exclude options to disable specific credential types inside the DefaultAzureCredentialFactory. Using the DefaultAzureCredential class allows someone to create an application that offers various authentication strategies without having to worry about the implementation inside the DefaultAzureCredentialFactory. When a user of that application wants to use a specific credential type this would mean the implementation would need to set all the Exclude options to true expect the specific one they want to use. But the downside of these Exclude options is that when a new credential type would be added this would mean that after an upgrade the implementation might include extra unwanted functionality.
It would be nice if a flagged enum could be used to specify which credential types are enabled but I suspect this was not done because this is not available in all languages the azure-sdk is written in. It looked like the original implementation used Include options instead of Exclude options but that was renamed after API review feedback in #8214. But I could not find why this was changed. I would like to propose the following mixed api:
public class DefaultAzureCredentialOptions
{
/// <summary>
/// Enables all credential types.
/// </summary>
public bool IncludeAllCredentialTypes { get; set; }
/// <summary>
/// Disables all credential types.
/// </summary>
public bool ExcludeAllCredentialTypes { get; set; }
/// <summary>
/// Enables EnvironmentCredential.
/// </summary>
public bool IncludeEnvironmentCredential { get; set; }
/// <summary>
/// Disables EnvironmentCredential.
/// </summary>
public bool ExcludeEnvironmentCredential { get; set; }
/// And do the same for the rest option the credential types.
}
I can also understand that this would pollute the api surface with all these extra options. If the team thinks this is a not a good idea I would request to at least add the ExcludeAllCredentialTypes option to disable everything at once.
Hi @dlemstra. Thanks for reaching out and we regret that you're experiencing difficulties. It is not recommended that you use DefaultAzureCredential for scenarios where you know the exact credential type that you'd like to use or when you have a consistent subset of credentials that you would always like to use. In these cases, using a more specific credential type is recommended.
Would you help us understand more about your end-to-end scenario and how you're using the credential?
Hi @dlemstra. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.
The application is an application that needs to authenticate with an azure api endpoint to perform an action for a user. The idea is to use DefaultAzureCredential by default making is possible to have multiple credential types available for the user. But also allow the user a command line option to specify which credential type they want to use. The user could for example add --type environment and then we would then only want to use the EnvironmentCredential. We could of course duplicate the creation that is done inside the DefaultAzureCredentialFactory but setting the excludes seemed like the easiest way to do this. The only downside to that approach is that there is no propertyy/method to disable everything except that single option.
Hi @dlemstra: I'm still not clear on the end-to-end flow of the application. It sounds like you may be using the Microsoft.Extensions.Azure package and using DI to inject your credential. If that's the case, you can substitute the default credential for the specific credential specified on the command line using the UseCredential method, as demonstrated in Dependency injection with the Azure SDK for .NET.
If that's not the case and you are directly creating the DefaultAzureCredential instance, then I apologize, but I'm still not understanding why it would be advantageous for you to configure the default credential rather than creating the specific credential requested. To illustrate what's in my head, something like:
TokenCredential appCred = args.TokenType switch
{
"environment" => new EnvironmentCredential(),
_ => new DefaultAzureCredential();
};
app.UseThisCredential(appCred);
Would you be willing to share some additional context to help us understand the end-to-end scenario? If possible, code snippets for illustration would be helpful.
Hi @dlemstra. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.
An example where DefaultAzureCredential is used as I described earlier is here: https://github.com/dotnet/sign/blob/1df56d5c09b004fec6b5413dc6c61183553589ec/src/Sign.Cli/AzureCredentialOptions.cs#L74. This allows the users of this command line application to authenticate with the Azure api through various methods without the authors of that application needing to maintain an implementation where various credential types are provided. If a new credential type gets added to the DefaultAzureCredentialFactory it will immediately become available in that application after an upgrade. But some user might want to specify which credential type they want to use. But with the current Exclude* options you will need to do something like this: https://github.com/Azure/trusted-signing-action?tab=readme-ov-file#exclude-credentials. It requires a user of that action to specify 6/7 excludes to enable a single credential type.
@dlemstra : Thanks for the additional context. Extrapolating to smaller focused snippets, it seems like what we're looking at is essentially a trade-off between using DefaultAzureCredential for all scenarios or managing different credential types directly. To extrapolate the context that you've shared and boil it down to a more focused snippet, I believe what we're looking at is something like the following.
With the requested feature:
var options = new DefaultCredentialOptions();
if (!string.IsNullOrEmpty(args.TokenType))
{
// Does not exist; used for illustration.
options.ExcludeAll = true;
}
// None of these options exist; used for illustration
switch (args.TokenType)
{
case "environment":
options.IncludeEnvironmentCredential = true;
break;
case "cli":
options.IncludeAzureCliCredential = true;
break;
// ... SNIP ...
default:
// We don't know what the requested credential is, so let's use them all.
options.IncludeAll = true;
break;
}
var credential = new DefaultAzureCredential(options);
app.UseThisCredential(appCred);
Using an explicit credential:
TokenCredential appCred = args.TokenType switch
{
"environment" => new EnvironmentCredential(),
"cli" => new AzureCliCredential();
// ... SNIP ...
// We don't know what the requested credential is, so let's use them all.
_ => new DefaultAzureCredential();
};
app.UseThisCredential(appCred);
Using a chained credential:
TokenCredential appCred = args.TokenType switch
{
"environment" => new EnvironmentCredential(),
"cli" => new AzureCliCredential();
// ... SNIP ...
// We don't know what the requested credential is, so let's use the local development set.
_ => new ChainedTokenCredential(
new AzureCliCredential(),
new AzureDeveloperCliCredential(),
new VisualStudioCredential());
};
app.UseThisCredential(appCred);
To be transparent, I don't see how the proposed changes to options really improves the flow of the code that would need to be written for the scenario. I'm also not sure that I believe it offers enough value for the complexity of having competing include/exclude semantics expand the public API nor that it follows the intended guidance for using DefaultAzureCredential.
However, I believe that we have enough understanding of the proposed changes and the end-to-end scenario at this point to discuss with the Azure.Identity development team across languages.
Thanks for considering this and I also understand that it might not be a good idea to add all this extra complexity. But please consider just adding something like ExcludeAll.
Hi @dlemstra - I agree with @jsquire 's assessment that adding these extra options doesn't really improve the intended scenario much, while adding additional complexity to the API surface.
Could you clarify what the intended behavior or ExcludeAll would be?
Hi @dlemstra. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.
The use case would be this:
- The command line application would use the
DefaultAzureCredentialFactorythroughDefaultAzureCredentialand that would work for most users. - With an option
--azure-credential-type shared-token-cachethe user can specify which credential type they want to use. And then the code of the command line application will do this:- Create a new
DefaultAzureCredentialinstance like we would do for normal usage. - Set
ExcludeAllto true to disable all credential types. - Set
ExcludeSharedTokenCacheCredentialtofalseto only enable this specific credential type.
- Create a new
Our current thinking is that we try to avoid adding more complexity to the already complex DefaultAzureCredentialOptions unless there is a clear need. For internal implementations, as in your example scenario, the alternatives outlined by @jsquire seem simpler all around.