azidentity: DefaultAzureCredentialOptions to allow opting out some of the auth methods

[magodo]

Feature Request

Currently, the DefaultAzureCredentialOptions will always try each defined auth method in the fixed order. Imagine I have a VM and want to run some Go code on it to interact with ARM using this SDK, it will always try the MSI (i.e. the VM's identity) instead of falling back to using Azure CLI (i.e. my user account). The request sent from my App will then fail since I didn't give enough role to the VM, which I deliberately want to avoid.

[chlowell]

We decided not to add such options because we want to keep the DefaultAzureCredential API simple to avoid overwhelming developers who are just getting started with Azure, and making the credential difficult to use and understand. The credential types used in the default chain are exported so you can use them directly. If you want to authenticate with the Azure CLI, you can simply call NewAzureCLICredential instead of NewDefaultAzureCredential. If you want to change the composition of the default chain, you can create a custom chain with NewChainedTokenCredential.

[ghost]

Hi @magodo. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text “/unresolve” to remove the “issue-addressed” label and continue the conversation.

[ghost]

Hi @magodo, since you haven’t asked that we “/unresolve” the issue, we’ll close this out. If you believe further discussion is needed, please add a comment “/unresolve” to reopen the issue.

[magodo]

@chlowell I have to disagree with you that DefaultAzureCredential is to avoid overwhelming developers who are just getting started with Azure, if you look at the option definition

azure-sdk-for-go/sdk/azidentity/default_azure_credential.go

Lines 23 to 36 in 1a91847

	type DefaultAzureCredentialOptions struct {
	azcore.ClientOptions
	// AdditionallyAllowedTenants specifies additional tenants for which the credential may acquire tokens. Add
	// the wildcard value "*" to allow the credential to acquire tokens for any tenant. This value can also be
	// set as a semicolon delimited list of tenants in the environment variable AZURE_ADDITIONALLY_ALLOWED_TENANTS.
	AdditionallyAllowedTenants []string
	// DisableInstanceDiscovery should be true for applications authenticating in disconnected or private clouds.
	// This skips a metadata request that will fail for such applications.
	DisableInstanceDiscovery bool
	// TenantID identifies the tenant the Azure CLI should authenticate in.
	// Defaults to the CLI's default tenant, which is typically the home tenant of the user logged in to the CLI.
	TenantID string
	}

Things like DisableInstanceDiscovery is far more complicated than an option like msiDisabled.

Apparently I can implement my own DefaultAzureCredential by copy-pasting 90% of the existing implementation and adding the logic I want. I don't see a reason why not just adding those toggles here, in a non-breaking manner?

[chlowell]

DisableInstanceDiscovery is an advanced option I'm not fond of, but without it applications on e.g. Azure Stack can't use the type at all. I don't know a similarly compelling use case for an option like DisableManagedIdentity. If I understand your scenario correctly, you want to decide at runtime whether to skip managed identity authentication with code like this (please correct me if I misunderstand):

opts := azidentity.DefaultAzureCredentialOptions{}
if condition {
	opts.DisableManagedIdentity = true
}
cred, err := azidentity.NewDefaultAzureCredential(&opts)

Here's a way to get the same result with the current API:

var cred azcore.TokenCredential
if condition {
	cred, err := azidentity.NewAzureCLICredential(nil)
} else {
	cred, err := azidentity.NewManagedIdentityCredential(nil)
}

Does that work for your application? I'm sorry to have implied in my first response that more code is necessary.

[github-actions]

Hi @magodo. 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.

[magodo]

@chlowell Yes, I could do that, whilst I still want to keep checking the environment variables for client certificate/secret auth method in the meanwhile. What I'm actually asking is kind of a togglable DefaultAzureCredential, which users can choose which methods to enable/disable via the option.

[RickWinter]

By design we do not allow modifying which credentials the DefaultAzureCredential uses. If you would like fine grained control over which credential is used and in which order, the proper approach is to use the ChainedTokenCredential.
https://github.com/Azure/azure-sdk-for-go/tree/main/sdk/azidentity#define-a-custom-authentication-flow-with-chainedtokencredential

[magodo]

@RickWinter Would you mind to point which design are your referring to? I didn't recall there is a design principle about how the default auth should be like in the Azure SDK design principle.

[magodo]

BTW, things like AdditionallyAllowedTenants are unwieldly designed and implemented in the default credential, which makes it consued to follow in a rewrite. The chain's name is private, the same for the implementation of the defaultCredentialErrorReporter 😓

[chlowell]

Would you mind to point which design are your referring to? I didn't recall there is a design principle about how the default auth should be like in the Azure SDK design principle.

There's a note in the readme (see the bottom of this section). I'll add something similar to the API documentation.

Why do you want to rewrite DefaultAzureCredential? ChainedTokenCredential enables combining credential types however you like. Expanding on my example above:

env, err := azidentity.NewEnvironmentCredential(nil)
if err != nil {
	// TODO: handle error
}
creds := []azcore.TokenCredential{env}
if condition {
	mi, err := azidentity.NewManagedIdentityCredential(nil)
	if err != nil {
		// TODO: handle error
	}
	creds = append(creds, mi)
}
cli, err := azidentity.NewAzureCLICredential(nil)
if err != nil {
	// TODO: handle error
}
creds = append(creds, cli)
cred, err := azidentity.NewChainedTokenCredential(creds, nil)

[magodo]

@chlowell Thanks for the illustration! But I noticed the // TODO: handle error part is actually hard to implement in a sensible way without touch the private types (like error). Also, you are simplifying the MSI part that the default one uses a timeWrapper, which also access some private error type in its implementation. Meanwhile, you are simplifying the NewEnvironmentCredential in regards of its usage of its additionallyAllowedTenants fields.